lkml.org 
[lkml]   [2010]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/4] hwmon: f71882fg: Add support for the Fintek F71808E
    Hi,

    On 08/01/2010 01:31 AM, Giel van Schijndel wrote:
    > On Wed, Mar 24, 2010 at 11:31:40 +0100, Hans de Goede wrote:
    >> On 03/24/2010 10:23, Giel van Schijndel wrote:
    >>> On Wed, Mar 24, 2010 at 09:25:08 +0100, Hans de Goede wrote:
    >>>> See comments inline.
    >>>>
    >>>> On 03/24/2010 12:12 AM, Giel van Schijndel wrote:
    >>>>> Allow device probing to recognise the Fintek F71808E.
    >>>>>
    >>>>> Sysfs interface:
    >>>>> * Fan/pwm control is the same as for F71889FG
    >>>>> * Temperature and voltage sensor handling is largely the same as for
    >>>>> the F71889FG
    >>>>> - Has one temperature sensor less (doesn't have temp3)
    >>>>> - Misses one voltage sensor (doesn't have V6, thus in6_input refers to
    >>>>> what in7_input refers for F71889FG)
    >>>>>
    >>>>> For the purpose of the sysfs interface fxxxx_in_temp_attr[] is split up
    >>>>> such that it can largely be reused.
    >>>>> ---
    >>>>> Documentation/hwmon/f71882fg | 4 ++
    >>>>> drivers/hwmon/Kconfig | 6 ++--
    >>>>> drivers/hwmon/f71882fg.c | 80 +++++++++++++++++++++++++++++++++++++----
    >>>>> 3 files changed, 79 insertions(+), 11 deletions(-)
    >>>>>
    >>>>> diff --git a/drivers/hwmon/f71882fg.c b/drivers/hwmon/f71882fg.c
    >>>>> index 25e1cad..b290b87 100644
    >>>>> --- a/drivers/hwmon/f71882fg.c
    >>>>> +++ b/drivers/hwmon/f71882fg.c
    >>>>> @@ -1974,8 +2002,27 @@ static int __devinit f71882fg_probe(struct platform_device *pdev)
    >>>>> ... snip ...
    >>>>> + /* fall through! */
    >>>>
    >>>> Ugh, please don't fall through, and then have an if below to only do
    >>>> some parts of the case falling through. This is quite confusing at
    >>>> first I thought your code was buggy I had to read it twice to notice
    >>>> the if. Instead just duplicate the following lines:
    >>>>> + err = f71882fg_create_sysfs_files(pdev,
    >>>>> + fxxxx_temp_attr,
    >>>>> + ARRAY_SIZE(fxxxx_temp_attr));
    >>>> In the f71862fg case, end the f71862fg case with a break and remove
    >>>> the if test from the f71808fg case.
    >>>>
    >>>>> + case f71808fg:
    >>>>> + if (data->type == f71808fg) {
    >>>>> + err = f71882fg_create_sysfs_files(pdev,
    >>>>> + f71808_in_attr,
    >>>>> + ARRAY_SIZE(f71808_in_attr));
    >>>>> + if (err)
    >>>>> + goto exit_unregister_sysfs;
    >>>>> + }
    >>>>> + err = f71882fg_create_sysfs_files(pdev,
    >>>>> + fxxxx_temp_attr,
    >>>>> + ARRAY_SIZE(fxxxx_temp_attr));
    >>>>> ... snip ...
    >>>
    >>> Ack. New and improved patch follows this line.
    >>> =====================================================================
    >>> hwmon: f71882fg: Add support for the Fintek F71808E
    >>
    >> This new version looks good to me:
    >> Acked-by: Hans de Goede<hdegoede@redhat.com>
    >
    > Thanks. Anyone else I need to poke in order to set this on track to
    > mainline?
    >

    No,

    Jean should have picked this up, I guess it has fallen through the cracks.

    I think it is probably best if you resend this patch and the watchdog patches
    re-based against the latest upstream rc, then I'll re-review them and ack
    them and Jean can then put them into his tree.

    Thanks & Regards,

    Hans


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