lkml.org 
[lkml]   [2014]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
    On Mon, 10 Feb 2014 18:01:59 +0000, Lee Jones wrote:
    > > > > > Might be worth taking the opportunity to swap out these magic numbers
    > > > > > now.
    > > > >
    > > > > There's nothing magic about them, they tell the driver how many fans
    > > > > each device supports. If you don't pass them as driver_data you'll have
    > > > > to derive them from the device name in the probe function.
    > > >
    > > > They're magic in that they're not easily identifiable. In the few
    > > > moments that I looked at the patch I assumed they were device
    > > > IDs. They should be clearly defined.
    > >
    > > They could have been device IDs, some drivers do that, and that would
    > > have been equally fine. driver_data can be anything. Best thing to do
    > > is to document it right above the device id array if you really find it
    > > confusing (I don't.) I don't know what else exactly you had in mind,
    > > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
    > > strike me as the best coding practice.
    >
    > On the contrary. Perhaps the nomenclature can be worked on a little,
    > but if I saw the aforementioned defines I would have known instantly
    > what was being defined without searching for co-located comments. Thus
    > elevating the requirement for me to even mention it. Even when we
    > use the .data element for very simple information such as device IDs
    > we do so with a #define.

    Right, you have a point here.

    I suppose it was deemed unneeded for a ~750 lines driver nobody really
    cared about. But if the driver is becoming more complex and popular
    then indeed it makes sense to clean it up a little. Starting with
    reordering functions to kill forward declarations ^^

    --
    Jean Delvare
    Suse L3 Support


    \
     
     \ /
      Last update: 2014-02-10 20:01    [W:4.090 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site