Re: [PATCH RESEND v2 3/7] mfd: cros_ec: Add cros_ec_lpc driver for x86 devices

[Javier Martinez Canillas]

Hello Lee,

On 01/20/2015 05:52 PM, Javier Martinez Canillas wrote:
>>> 
>>> Now, all those drivers may be wrong and the buses don't belong to the mfd
>>> subsystem but then I think we need to document that since it seems that is
>>> the correct way to do it just by looking at the other drivers.
>> 
>> I don't think the drivers you mentioned above do anything practical.
>> For instance, they are not SPI/IC2/etc drivers.  They should only
>> offer some abstraction layers which are used to communicate with the
>> device.  The driver you are submitting looks a lot more like a device
>> driver, which should live somewhere else.  Don't ask me where though,
>> I'm not even sure what a Low Pin Controller does.
>> 
> 
> The driver added by $subject doesn't really do anything practical either.
> LPC [0] is just another transport method like i2c or spi that is used on
> x86 Chromebooks to access the Embedded Controller.
> 
> So the driver is really not that different than the cros_ec_{i2c,spi}.c
> drivers.
> 
> Best regards,
> Javier
> 
> [0]: http://en.wikipedia.org/wiki/Low_Pin_Count
> 

Maybe the problem is that the commit message didn't explain this clearly?
I took the patch from the ChromiumOS tree verbatim but I can reword the
commit message to add something like this instead:

Author: Bill Richardson <wfrichar@xxxxxxxxxxxx>
Date:   Tue Dec 23 15:13:23 2014 +0100

    mfd: cros_ec: Add cros_ec_lpc driver for x86 devices
    
    Chromebooks have an Embedded Controller (EC) that is used to
    implement various functions such as keyboard, power and battery.
    
    The AP can communicate with the EC through different bus types
    such as I2C, SPI or LPC.
    
    The cros_ec mfd driver is then composed of a core driver that
    register the sub-devices as mfd cells and provide a high level
    communication interface that is used by the rest of the kernel
    and bus specific interfaces modules.
    
    Each connection method then has its own driver, which register
    with the EC driver interface-agnostic interface.
    
    Currently, there are drivers to communicate with the EC over
    I2C and SPI and this driver adds support for LPC.
    
    Signed-off-by: Bill Richardson <wfrichar@xxxxxxxxxxxx>
    Signed-off-by: Javier Martinez Canillas <javier.martinez@xxxxxxxxxxxxxxx>


If you agree with this commit message then I can change it on the next revision
but really $subject doesn't do anything special besides providing a transport
method to access the mfd device using a LPC bus.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/