On Tue, 18 Aug 2015, Petr Cvek wrote:
> Dne 18.8.2015 v 08:52 Lee Jones napsal(a):
> > On Tue, 18 Aug 2015, Petr Cvek wrote:
> >
> >> Fix register definitions and prepare structures for new leds-pasic3
> >> driver.
> >>
> >> Signed-off-by: Petr Cvek <petr.cvek@xxxxxx>
> >> ---
> >> drivers/mfd/htc-pasic3.c | 54 ++++++++++++++++-----------
> >> include/linux/mfd/htc-pasic3.h | 85 +++++++++++++++++++++++++++++++-----------
> >> 2 files changed, 97 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/drivers/mfd/htc-pasic3.c b/drivers/mfd/htc-pasic3.c
> >> index e88d4f6..16e156d 100644
> >> --- a/drivers/mfd/htc-pasic3.c
> >> +++ b/drivers/mfd/htc-pasic3.c
> >> @@ -3,6 +3,9 @@
> >> *
> >> * Copyright (C) 2006 Philipp Zabel <philipp.zabel@xxxxxxxxx>
> >> *
> >> + * 2015: Added registers for LED and RESET, see htc-pasic3.h
> >> + * -- Petr Cvek
> >> + *
> >
> > This is pretty unconventional.
> >
> > Please look to see what others have done.
>
> LED support: Petr Cvek <petr.cvek@xxxxxx>
We can see what changes/support you added in the Git log. No need to
reflect that here at all. If everyone did that, the headers would be
a complete mess.
Just:
* Copyright (C) 2015 Petr Cvek <petr.cvek@xxxxxx>
... will do fine.
> >> @@ -130,6 +140,7 @@ static int __init pasic3_probe(struct platform_device *pdev)
> >> struct device *dev = &pdev->dev;
> >> struct pasic3_data *asic;
> >> struct resource *r;
> >> + struct pasic3_leds_pdata *leds_pdata;
> >> int ret;
> >> int irq = 0;
> >>
> >> @@ -162,6 +173,8 @@ static int __init pasic3_probe(struct platform_device *pdev)
> >> /* calculate bus shift from mem resource */
> >> asic->bus_shift = (resource_size(r) - 5) >> 3;
> >>
> >> + spin_lock_init(&asic->lock);
> >> +
> >> if (pdata && pdata->clock_rate) {
> >> ds1wm_pdata.clock_rate = pdata->clock_rate;
> >> /* the first 5 PASIC3 registers control the DS1WM */
> >> @@ -172,13 +185,12 @@ static int __init pasic3_probe(struct platform_device *pdev)
> >> dev_warn(dev, "failed to register DS1WM\n");
> >> }
> >>
> >> - if (pdata && pdata->led_pdata) {
> >> - led_cell.platform_data = pdata->led_pdata;
> >> - led_cell.pdata_size = sizeof(struct pasic3_leds_machinfo);
> >> - ret = mfd_add_devices(&pdev->dev, pdev->id, &led_cell, 1, r,
> >> - 0, NULL);
> >> - if (ret < 0)
> >> - dev_warn(dev, "failed to register LED device\n");
> >> + if (pdata && pdata->pasic3_leds) {
> >> + leds_pdata = dev_get_platdata(&pdata->pasic3_leds->dev);
> >
> > Whoa! You're passing a pointer to a device through pdata?
> >
> > I don't like that at all. Please explain.
>
> >
> >> + if (leds_pdata) {
> >> + leds_pdata->mfd_dev = dev;
> >> + platform_device_register(pdata->pasic3_leds);
> >
> > What's the idea here?
>
> Actually, I don't know how to do this in a clean way as
> pasic3_read_register() and pasic3_write_register() require device
> struct to pass address for accessing the registers. Only way would
> be to rewrite all functions which call pasic3_*_register() (removing
> struct device *dev and change it to struct pasic3_data *asic).
I'm still not entirely sure what the problem is.
Why are you calling platform_device_register() instead of using the
MFD API?
Where does the 'struct device' actually get loaded into pdata?
--
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-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
