[lkml]   [2015]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 2/2] net: dsa: register hwmon for any provided function
Hi Guenter,

> >>> switch (index) {
> >>> + case 0: /* temp1_input */
> >>> + if (drv->get_temp)
> >>> + mode |= S_IRUGO;
> >>
> >> This should be mandatory. Sorry, I don't really understand what you
> >> are trying to accomplish here.
> >>
> >> Can you give me a real world example where a chip would support
> >> setting a limit but not reading it ?
> >
> > I have no such example. I just did not see why this couldn't be
> > allowed (e.g. setting only set_temp_limit and get_temp_alarm looks
> > fine to me). But if you say that get_temp should be mandatory, I'm
> > OK with that.
> >
> write-only attributes are not defined in the hwmon ABI. If the
> 'sensors' command encounters such an attribute, it will create an
> error message each time it executes. That doesn't sound very useful to
> me.

Ok, good to know.

> If a chip - for whatever reason - does not have a limit register but
> an alarm register or flag, its temperature limit is usually hard-coded
> and can be reported this way (the AMD temperature sensor driver does
> this, for example). If there is ever a need to support the
> alarm-register-only situation for some odd reason, we can add the code
> at the time. For now, it just seems to me that you are adding
> complexity to solve some theoretic problem which is very unlikely to
> occur in the real world.

You're right, this change is not necessary.

> > The primary goal of this patchset was to use DEVICE_ATTR_RW to
> > declare temp1_max, instead of reflecting the minimal permissions
> > needed.
> >
> Then why don't you just do that and nothing else ? The goal should be
> to simplify code, not to make it more complicated. If the result isn't
> less code, I don't think it is worth it.

Indeed, I'll rework this patch then.

Thanks for the review,

 \ /
  Last update: 2015-04-17 17:01    [W:0.079 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site