Messages in this thread | | | Date | Thu, 28 May 2015 15:26:09 +0100 | From | Lee Jones <> | Subject | Re: [PATCH v3 6/7] mfd: cros_ec: Support multiple EC in a system |
| |
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@chromium.org> > >> > >> 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@chromium.org> > >> Reviewed-by: Dmitry Torokhov <dtor@chromium.org> > >> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > >> --- > >> > >> 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
| |