On Thu, 28 May 2015, Javier Martinez Canillas wrote: > Hello Lee, > > Thanks a lot for your feedback. > > On 05/27/2015 11:11 AM, Lee Jones wrote: > > On Fri, 22 May 2015, Javier Martinez Canillas wrote: > > > >> From: Gwendal Grignou <gwendal@xxxxxxxxxxxx> > >> > >> Chromebooks can have more than one Embedded Controller so the > >> cros_ec device id has to be incremented for each EC registered. > >> > >> Add code to handle multiple EC. First ec found is cros-ec0, > >> second cros-ec1 and so on. > >> > >> Add a new structure to represent multiple EC as different char > >> devices (e.g: /dev/cros_ec, /dev/cros_pd). It connects to > >> cros_ec_device and allows sysfs inferface for cros_pd. > >> > >> Also reduce number of allocated objects, make chromeos sysfs > >> class object a static and add refcounting to prevent object > >> deletion while command is in progress. > >> > >> Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx> > >> Reviewed-by: Dmitry Torokhov <dtor@xxxxxxxxxxxx> > >> Signed-off-by: Javier Martinez Canillas <javier.martinez@xxxxxxxxxxxxxxx> > >> --- > >> > >> Changes since v2: None > >> > >> Changes since v1: > >> - Squash patch that adds support to represent EC's as different > >> char devices (e.g: /dev/cros_ec, /dev/cros_pd): > >> https://chromium-review.googlesource.com/#/c/217297/ > >> Suggested by Gwendal Grignou > >> - Use cros_ec instead of cros-ec in the subject line to be consistent. > >> Suggested by Gwendal Grignou > >> --- > >> drivers/input/keyboard/cros_ec_keyb.c | 2 +- > >> drivers/mfd/cros_ec.c | 66 +++++++++++++-- > >> drivers/mfd/cros_ec_i2c.c | 1 - > >> drivers/mfd/cros_ec_spi.c | 1 - > >> drivers/platform/chrome/cros_ec_dev.c | 128 ++++++++++++++++++++--------- > >> drivers/platform/chrome/cros_ec_dev.h | 7 -- > >> drivers/platform/chrome/cros_ec_lightbar.c | 75 +++++++++-------- > >> drivers/platform/chrome/cros_ec_lpc.c | 1 - > >> drivers/platform/chrome/cros_ec_sysfs.c | 48 +++++------ > >> include/linux/mfd/cros_ec.h | 44 ++++++++-- > >> 10 files changed, 247 insertions(+), 126 deletions(-) [...] > >> @@ -52,14 +88,28 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > >> > >> cros_ec_query_all(ec_dev); > >> > >> - err = mfd_add_devices(dev, 0, cros_devs, > >> - ARRAY_SIZE(cros_devs), > >> - NULL, ec_dev->irq, NULL); > >> + err = cros_ec_dev_register(ec_dev, dev_id++, 0); > >> if (err) { > >> - dev_err(dev, "failed to add mfd devices\n"); > >> + dev_err(dev, "failed to add ec\n"); > >> return err; > >> } > >> > >> + if (ec_dev->max_passthru) { > >> + /* > >> + * Register a PD device as well on top of this device. > >> + * We make the following assumptions: > >> + * - behind an EC, we have a pd > >> + * - only one device added. > >> + * - the EC is responsive at init time (it is not true for a > >> + * sensor hub. > >> + */ > >> + err = cros_ec_dev_register(ec_dev, dev_id++, 1); > > > > I don't really like this devidx business. Just keep it simple and > > define more than one mfd_cell structure. > > I explained to you that this is done because the number of cells depends on > the system. I can have an array of mfd_cell structures and use the index to > register but I don't think that is easier to understand. Keep it simple. Create a static struct for each and: mfd_add_devices(ec_cell) if (ec_dev->max_passthru) mfd_add_devices(ec_pd_cell) > >> + if (err) { > >> + dev_err(dev, "failed to add additional ec\n"); > >> + return err; > >> + } > >> + } > >> + > >> if (IS_ENABLED(CONFIG_OF) && dev->of_node) { > >> err = of_platform_populate(dev->of_node, NULL, NULL, dev); > >> if (err) { > > > > Best regards, > Javier -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |