linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Simple MFD driver example
       [not found] <CAOMwXhN5UfR=Y8V3i=A2zSCv7V9sRDd0Z+D1AQFahvRS-mV5Fw@mail.gmail.com>
@ 2013-12-16  8:25 ` Lee Jones
  2013-12-16 14:00   ` Laszlo Papp
  0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2013-12-16  8:25 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: sameo, linux-kernel

> I have been trying to write about 50-100 LOC for 1-2 weeks now. I have
> a fairly simple use case for MFD where I need to have gpio and hwmon
> functionality for a GPIO.
> 
> I read the mfd drivers inside and out by now, but it looks a bit
> inconsistent to me. I am not sure what to follow etc. The
> documentation seems to be virtually missing which makes my life and
> involvement even more difficult. :(
> 
> May I ask for some pointer how to do all this? It would be nice to see
> a minimal example where the parent does not do anything else than just
> enumerating the sub-device drivers. Is that possible with an empty
> probe callback?
> 
> If I understand the regmap correctly (again, it is possible I do not),
> then I would not need this. But in general, it is really hard to deal
> with the linux kernel in this regard when basic documentations are
> missing.
> 
> I hope this would change over time for the posterity. It is a pity
> that contributors do not join probably due to this. It is a bit
> chaotic, but I hope it can be changed in the future.
> 
> Apologies for my frustration...

Due to the nature of the MFD subsystem, there isn't a simple use-case
which you can use as a template. It's completely dependent on the
device you are trying to enable. There are lots of well written
drivers in the subsystem that you can use as good examples though.

Which device are you trying to write a driver for? Is there a
datasheet you can point to?

NB: When emailing subsystem maintainers for help, please don't forget
to CC the relevant Mailing Lists. All lists can be found in the
MAINTAINERS file, which is parsed by <kernel>/scripts/get_maintainer.pl.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-16  8:25 ` Simple MFD driver example Lee Jones
@ 2013-12-16 14:00   ` Laszlo Papp
  2013-12-16 15:05     ` Lee Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Papp @ 2013-12-16 14:00 UTC (permalink / raw)
  To: Lee Jones; +Cc: sameo, linux-kernel

Well, if you ask me the MFD subsystem is underdocumented, and there is
some inconsistency between the drivers. It is hard to take any of them
as a starting point, at least for me. Currently, I wrote a simple one
from scratch which I will post soon.

On Mon, Dec 16, 2013 at 8:25 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> I have been trying to write about 50-100 LOC for 1-2 weeks now. I have
>> a fairly simple use case for MFD where I need to have gpio and hwmon
>> functionality for a GPIO.
>>
>> I read the mfd drivers inside and out by now, but it looks a bit
>> inconsistent to me. I am not sure what to follow etc. The
>> documentation seems to be virtually missing which makes my life and
>> involvement even more difficult. :(
>>
>> May I ask for some pointer how to do all this? It would be nice to see
>> a minimal example where the parent does not do anything else than just
>> enumerating the sub-device drivers. Is that possible with an empty
>> probe callback?
>>
>> If I understand the regmap correctly (again, it is possible I do not),
>> then I would not need this. But in general, it is really hard to deal
>> with the linux kernel in this regard when basic documentations are
>> missing.
>>
>> I hope this would change over time for the posterity. It is a pity
>> that contributors do not join probably due to this. It is a bit
>> chaotic, but I hope it can be changed in the future.
>>
>> Apologies for my frustration...
>
> Due to the nature of the MFD subsystem, there isn't a simple use-case
> which you can use as a template. It's completely dependent on the
> device you are trying to enable. There are lots of well written
> drivers in the subsystem that you can use as good examples though.
>
> Which device are you trying to write a driver for? Is there a
> datasheet you can point to?
>
> NB: When emailing subsystem maintainers for help, please don't forget
> to CC the relevant Mailing Lists. All lists can be found in the
> MAINTAINERS file, which is parsed by <kernel>/scripts/get_maintainer.pl.
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-16 14:00   ` Laszlo Papp
@ 2013-12-16 15:05     ` Lee Jones
  2013-12-16 16:39       ` Laszlo Papp
  0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2013-12-16 15:05 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: sameo, linux-kernel

Please refrain from top posting.

Responses should appear under quoted lines, which they reference.

> >> I have been trying to write about 50-100 LOC for 1-2 weeks now. I have
> >> a fairly simple use case for MFD where I need to have gpio and hwmon
> >> functionality for a GPIO.
> >>
> >> I read the mfd drivers inside and out by now, but it looks a bit
> >> inconsistent to me. I am not sure what to follow etc. The
> >> documentation seems to be virtually missing which makes my life and
> >> involvement even more difficult. :(
> >>
> >> May I ask for some pointer how to do all this? It would be nice to see
> >> a minimal example where the parent does not do anything else than just
> >> enumerating the sub-device drivers. Is that possible with an empty
> >> probe callback?
> >>
> >> If I understand the regmap correctly (again, it is possible I do not),
> >> then I would not need this. But in general, it is really hard to deal
> >> with the linux kernel in this regard when basic documentations are
> >> missing.
> >>
> >> I hope this would change over time for the posterity. It is a pity
> >> that contributors do not join probably due to this. It is a bit
> >> chaotic, but I hope it can be changed in the future.
> >>
> >> Apologies for my frustration...
> >
> > Due to the nature of the MFD subsystem, there isn't a simple use-case
> > which you can use as a template. It's completely dependent on the
> > device you are trying to enable. There are lots of well written
> > drivers in the subsystem that you can use as good examples though.

> Well, if you ask me the MFD subsystem is underdocumented, and there is
> some inconsistency between the drivers. It is hard to take any of them
> as a starting point, at least for me.

Feel free to write some. I'm always happy to take helpful patches.

> Currently, I wrote a simple one from scratch which I will post soon.

Okay great.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-16 15:05     ` Lee Jones
@ 2013-12-16 16:39       ` Laszlo Papp
  2013-12-16 16:48         ` Lee Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Papp @ 2013-12-16 16:39 UTC (permalink / raw)
  To: Lee Jones; +Cc: sameo, LKML

On Mon, Dec 16, 2013 at 3:05 PM, Lee Jones <lee.jones@linaro.org> wrote:
> Please refrain from top posting.
>
> Responses should appear under quoted lines, which they reference.

I intentionally did not reply to a specific quote, but to the email as a whole!

>> >> I have been trying to write about 50-100 LOC for 1-2 weeks now. I have
>> >> a fairly simple use case for MFD where I need to have gpio and hwmon
>> >> functionality for a GPIO.
>> >>
>> >> I read the mfd drivers inside and out by now, but it looks a bit
>> >> inconsistent to me. I am not sure what to follow etc. The
>> >> documentation seems to be virtually missing which makes my life and
>> >> involvement even more difficult. :(
>> >>
>> >> May I ask for some pointer how to do all this? It would be nice to see
>> >> a minimal example where the parent does not do anything else than just
>> >> enumerating the sub-device drivers. Is that possible with an empty
>> >> probe callback?
>> >>
>> >> If I understand the regmap correctly (again, it is possible I do not),
>> >> then I would not need this. But in general, it is really hard to deal
>> >> with the linux kernel in this regard when basic documentations are
>> >> missing.
>> >>
>> >> I hope this would change over time for the posterity. It is a pity
>> >> that contributors do not join probably due to this. It is a bit
>> >> chaotic, but I hope it can be changed in the future.
>> >>
>> >> Apologies for my frustration...
>> >
>> > Due to the nature of the MFD subsystem, there isn't a simple use-case
>> > which you can use as a template. It's completely dependent on the
>> > device you are trying to enable. There are lots of well written
>> > drivers in the subsystem that you can use as good examples though.
>
>> Well, if you ask me the MFD subsystem is underdocumented, and there is
>> some inconsistency between the drivers. It is hard to take any of them
>> as a starting point, at least for me.
>
> Feel free to write some. I'm always happy to take helpful patches.

I am not the right person for documentating everything here,
especially when I do not even understand basics... I am just trying to
figure out my way here. It was more like a hint that this area could
take some love. At least, I would personally appreciate it a lot. It
is currently somewhat difficult to get involved.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-16 16:39       ` Laszlo Papp
@ 2013-12-16 16:48         ` Lee Jones
  2013-12-17 17:09           ` Laszlo Papp
  0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2013-12-16 16:48 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: sameo, LKML

> > Please refrain from top posting.
> >
> > Responses should appear under quoted lines, which they reference.
> 
> I intentionally did not reply to a specific quote, but to the email as a whole!

Then you should have placed your response underneath the whole email,
not on top. If you're going to continue contributing to Mainline,
please find yourself a better email client. I assume you're using
Gmail at the moment, which is not suitable for this purpose.

> >> >> I have been trying to write about 50-100 LOC for 1-2 weeks now. I have
> >> >> a fairly simple use case for MFD where I need to have gpio and hwmon
> >> >> functionality for a GPIO.
> >> >>
> >> >> I read the mfd drivers inside and out by now, but it looks a bit
> >> >> inconsistent to me. I am not sure what to follow etc. The
> >> >> documentation seems to be virtually missing which makes my life and
> >> >> involvement even more difficult. :(
> >> >>
> >> >> May I ask for some pointer how to do all this? It would be nice to see
> >> >> a minimal example where the parent does not do anything else than just
> >> >> enumerating the sub-device drivers. Is that possible with an empty
> >> >> probe callback?
> >> >>
> >> >> If I understand the regmap correctly (again, it is possible I do not),
> >> >> then I would not need this. But in general, it is really hard to deal
> >> >> with the linux kernel in this regard when basic documentations are
> >> >> missing.
> >> >>
> >> >> I hope this would change over time for the posterity. It is a pity
> >> >> that contributors do not join probably due to this. It is a bit
> >> >> chaotic, but I hope it can be changed in the future.
> >> >>
> >> >> Apologies for my frustration...
> >> >
> >> > Due to the nature of the MFD subsystem, there isn't a simple use-case
> >> > which you can use as a template. It's completely dependent on the
> >> > device you are trying to enable. There are lots of well written
> >> > drivers in the subsystem that you can use as good examples though.
> >
> >> Well, if you ask me the MFD subsystem is underdocumented, and there is
> >> some inconsistency between the drivers. It is hard to take any of them
> >> as a starting point, at least for me.
> >
> > Feel free to write some. I'm always happy to take helpful patches.
> 
> I am not the right person for documentating everything here,
> especially when I do not even understand basics... I am just trying to
> figure out my way here. It was more like a hint that this area could
> take some love. At least, I would personally appreciate it a lot. It
> is currently somewhat difficult to get involved.

There isn't any documentation that would help you I fear. I'm trying
to help you, but you haven't answered my previous question.

Do you have a datasheet for the device that you're trying to enable?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-16 16:48         ` Lee Jones
@ 2013-12-17 17:09           ` Laszlo Papp
  2013-12-18  9:16             ` Lee Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Papp @ 2013-12-17 17:09 UTC (permalink / raw)
  To: Lee Jones; +Cc: sameo, LKML

On Mon, Dec 16, 2013 at 4:48 PM, Lee Jones <lee.jones@linaro.org> wrote:
> There isn't any documentation that would help you I fear. I'm trying
> to help you, but you haven't answered my previous question.

I did intend to answer your question when I was writing that I would
send some code. See it at the bottom of the email.

> Do you have a datasheet for the device that you're trying to enable?

datasheets.maximintegrated.com/en/ds/MAX6650-MAX6651.pdf

hwmon is already available in the kernel (for which my simple patch
was rejected). I have just sent out a gpio driver for the gpio bits,
and see the mfd driver code below.

/*
 * Device access for MAX6651
 *
 * Copyright(c) 2013 Polatis Ltd.
 *
 * Author: Laszlo Papp <laszlo.papp@polatis.com>
 *
 *  This program is free software; you can redistribute  it and/or modify it
 *  under  the terms of  the GNU General  Public License as published by the
 *  Free Software Foundation;  either version 2 of the  License, or (at your
 *  option) any later version.
 */

#include <linux/device.h>
#include <linux/delay.h>
#include <linux/input.h>
#include <linux/interrupt.h>
#include <linux/mfd/core.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/i2c.h>

#include <linux/mfd/max6651.h>

static struct mfd_cell max6651_devs[] = {
    { .name = "max6651-gpio", },
    { .name = "max6650", },
};

static int max6651_i2c_probe(struct i2c_client *i2c,
                            const struct i2c_device_id *id)
{
        struct max6651_dev *max6651;
        int ret = 0;

        max6651 = kzalloc(sizeof(struct max6651_dev), GFP_KERNEL);
        if (max6651 == NULL)
                return -ENOMEM;

        i2c_set_clientdata(i2c, max6651);
        max6651->dev = &i2c->dev;

        mutex_init(&max6651->iolock);

        ret = mfd_add_devices(max6651->dev, -1, max6651_devs,
                        ARRAY_SIZE(max6651_devs),
                        NULL, 0);

        if (ret < 0) {
        dev_err(max6651->dev, "cannot add mfd cells\n");
                goto err_mfd;
    }

        return ret;

err_mfd:
        mfd_remove_devices(max6651->dev);
err:
        kfree(max6651);
        return ret;
}

static int max6651_i2c_remove(struct i2c_client *i2c)
{
    struct max6651_dev *max6651 = i2c_get_clientdata(i2c);

    mfd_remove_devices(max6651->dev);

    return 0;
}

static const struct i2c_device_id max6651_i2c_id[] = {
    { "max6650", TYPE_MAX6650 },
    { "max6651", TYPE_MAX6651 },
    { }
};
MODULE_DEVICE_TABLE(i2c, max6651_i2c_id);

static struct i2c_driver max6651_i2c_driver = {
    .driver = {
           .name = "max6651",
           .owner = THIS_MODULE,
    },
    .probe = max6651_i2c_probe,
    .remove = max6651_i2c_remove,
    .id_table = max6651_i2c_id,
};

static int __init max6651_i2c_init(void)
{
    return i2c_add_driver(&max6651_i2c_driver);
}
/* init early so consumer devices can complete system boot */
subsys_initcall(max6651_i2c_init);

static void __exit max6651_i2c_exit(void)
{
    i2c_del_driver(&max6651_i2c_driver);
}
module_exit(max6651_i2c_exit);

MODULE_AUTHOR("Laszlo Papp <laszlo.papp@polatis.com>");
MODULE_DESCRIPTION("MAX6651 MFD");
MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-17 17:09           ` Laszlo Papp
@ 2013-12-18  9:16             ` Lee Jones
  2013-12-18 10:42               ` Laszlo Papp
  0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2013-12-18  9:16 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: sameo, LKML

> On Mon, Dec 16, 2013 at 4:48 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > There isn't any documentation that would help you I fear. I'm trying
> > to help you, but you haven't answered my previous question.
> 
> I did intend to answer your question when I was writing that I would
> send some code. See it at the bottom of the email.
> 
> > Do you have a datasheet for the device that you're trying to enable?
> 
> datasheets.maximintegrated.com/en/ds/MAX6650-MAX6651.pdf

What has alluded you to the fact that this is an MFD device?

a) It doesn't look like one to me

b) This device looks like it's already supported in Mainline

> hwmon is already available in the kernel (for which my simple patch
> was rejected). I have just sent out a gpio driver for the gpio bits,
> and see the mfd driver code below.

I'm not sure why it was rejected, but the MAX6651 already looks
supported to me in:
  drivers/hwmon/max6650.c.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-18  9:16             ` Lee Jones
@ 2013-12-18 10:42               ` Laszlo Papp
  2013-12-18 11:12                 ` Lee Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Papp @ 2013-12-18 10:42 UTC (permalink / raw)
  To: Lee Jones; +Cc: sameo, LKML, Linus Walleij, Guenter Roeck

The MAX6650/6651 chip has multiple functionality:

* fan/hwmon

* gpio

* alarm, etc.

What you eventually see in hwmon is only a subset of all the features
the IC provides. You may want to read this thread:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg536509.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-18 10:42               ` Laszlo Papp
@ 2013-12-18 11:12                 ` Lee Jones
  2013-12-18 11:17                   ` Laszlo Papp
  0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2013-12-18 11:12 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: sameo, LKML, Linus Walleij, Guenter Roeck

> The MAX6650/6651 chip has multiple functionality:
> 
> * fan/hwmon
> 
> * gpio
> 
> * alarm, etc.
> 
> What you eventually see in hwmon is only a subset of all the features
> the IC provides. You may want to read this thread:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg536509.html

Okay, so the best thing to do is send out the entire patch-set at
once, CC'ing each of the maintainers on every patch so we can all see
how this thing fits together.

Don't forget to create a cover-letter and each patch should be
in-reply to the cover letter. If you haven't done this before,
practise by sending it to yourself until it looks correct rather than
spamming the list.

It should look like this on this list:

[PATCH 00/03] Subject ...
 |-> [PATCH 01/03] mfd: Description ...
 |-> [PATCH 02/03] hwmon: Description ...
 |-> [PATCH 03/03] gpio: Description ...


-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-18 11:12                 ` Lee Jones
@ 2013-12-18 11:17                   ` Laszlo Papp
  2013-12-18 11:34                     ` Lee Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Papp @ 2013-12-18 11:17 UTC (permalink / raw)
  To: Lee Jones; +Cc: sameo, LKML, Linus Walleij, Guenter Roeck

On Wed, Dec 18, 2013 at 11:12 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> The MAX6650/6651 chip has multiple functionality:
>>
>> * fan/hwmon
>>
>> * gpio
>>
>> * alarm, etc.
>>
>> What you eventually see in hwmon is only a subset of all the features
>> the IC provides. You may want to read this thread:
>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg536509.html
>
> Okay, so the best thing to do is send out the entire patch-set at
> once, CC'ing each of the maintainers on every patch so we can all see
> how this thing fits together.

Well, I am not even sure currently where to head with the MFD bits and
its children subdevices currently....

I would appreciate any direction. Yesterday, I was told on IRC, I
would need to switch from i2c to platform drivers for the hwmon and
gpio parts, but looking at some existing mfd driver code and their
children drivers, I do not see it like that.

I have already sent out the gpio driver yesterday which works fine on
its own: http://www.spinics.net/lists/kernel/msg1655805.html

Could you please guide me into the right direction what I need to
change once we have standalone drivers, and they should be glued
together? I thought adding an abstraction with the mfd layer would be
sufficient, but apparently, that is not enough.

Practically speaking, I am confused since if I needed to change the
existing drivers, that means I could potentially break the interface
for the existing users if the drivers stop working on their own, but
then again, I am such a newbie that I would greatly appreciate some
pointers.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-18 11:17                   ` Laszlo Papp
@ 2013-12-18 11:34                     ` Lee Jones
  2013-12-18 11:49                       ` Laszlo Papp
  0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2013-12-18 11:34 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: sameo, LKML, Linus Walleij, Guenter Roeck

> >> What you eventually see in hwmon is only a subset of all the features
> >> the IC provides. You may want to read this thread:
> >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg536509.html
> >
> > Okay, so the best thing to do is send out the entire patch-set at
> > once, CC'ing each of the maintainers on every patch so we can all see
> > how this thing fits together.
> 
> Well, I am not even sure currently where to head with the MFD bits and
> its children subdevices currently....
> 
> I would appreciate any direction. Yesterday, I was told on IRC, I
> would need to switch from i2c to platform drivers for the hwmon and
> gpio parts, but looking at some existing mfd driver code and their
> children drivers, I do not see it like that.
> 
> I have already sent out the gpio driver yesterday which works fine on
> its own: http://www.spinics.net/lists/kernel/msg1655805.html

This is going to need a lot of work.

Did you run the patch through `./scripts/checkpatch.pl` before
submitting?

> Could you please guide me into the right direction what I need to
> change once we have standalone drivers, and they should be glued
> together? I thought adding an abstraction with the mfd layer would be
> sufficient, but apparently, that is not enough.
> 
> Practically speaking, I am confused since if I needed to change the
> existing drivers, that means I could potentially break the interface
> for the existing users if the drivers stop working on their own, but
> then again, I am such a newbie that I would greatly appreciate some
> pointers.

The MFD subsystem is quite simple to use. I'm taken aback that this is
your major stumbling block. Read though the mfd_add_device(s)() calls
to see what it expects. The rest is childs play.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-18 11:34                     ` Lee Jones
@ 2013-12-18 11:49                       ` Laszlo Papp
  2013-12-18 11:59                         ` Lee Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Papp @ 2013-12-18 11:49 UTC (permalink / raw)
  To: Lee Jones; +Cc: sameo, LKML, Linus Walleij, Guenter Roeck

On Wed, Dec 18, 2013 at 11:34 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> What you eventually see in hwmon is only a subset of all the features
>> >> the IC provides. You may want to read this thread:
>> >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg536509.html
>> >
>> > Okay, so the best thing to do is send out the entire patch-set at
>> > once, CC'ing each of the maintainers on every patch so we can all see
>> > how this thing fits together.
>>
>> Well, I am not even sure currently where to head with the MFD bits and
>> its children subdevices currently....
>>
>> I would appreciate any direction. Yesterday, I was told on IRC, I
>> would need to switch from i2c to platform drivers for the hwmon and
>> gpio parts, but looking at some existing mfd driver code and their
>> children drivers, I do not see it like that.
>>
>> I have already sent out the gpio driver yesterday which works fine on
>> its own: http://www.spinics.net/lists/kernel/msg1655805.html
>
> This is going to need a lot of work.
>
> Did you run the patch through `./scripts/checkpatch.pl` before
> submitting?

Of course, there has been zero errors and warnings. Eventually, I even
ran the Lindent. Actual feedback is welcome for sure.

>> Could you please guide me into the right direction what I need to
>> change once we have standalone drivers, and they should be glued
>> together? I thought adding an abstraction with the mfd layer would be
>> sufficient, but apparently, that is not enough.
>>
>> Practically speaking, I am confused since if I needed to change the
>> existing drivers, that means I could potentially break the interface
>> for the existing users if the drivers stop working on their own, but
>> then again, I am such a newbie that I would greatly appreciate some
>> pointers.
>
> The MFD subsystem is quite simple to use. I'm taken aback that this is
> your major stumbling block. Read though the mfd_add_device(s)() calls
> to see what it expects. The rest is childs play.

Yeah, I have taken, but that does not still explain the consistency I
mentioned above. Some children do not conform the "platform" driver
suggestion I was told.

Also, what about the actual MFD code submitted? Anything to modify in
there? Could you please comment on that, or is the direction of it
good enough for me to submit it as a real patch at this stage?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-18 11:49                       ` Laszlo Papp
@ 2013-12-18 11:59                         ` Lee Jones
  2013-12-18 12:01                           ` Laszlo Papp
  0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2013-12-18 11:59 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: sameo, LKML, Linus Walleij, Guenter Roeck

On Wed, 18 Dec 2013, Laszlo Papp wrote:

> On Wed, Dec 18, 2013 at 11:34 AM, Lee Jones <lee.jones@linaro.org> wrote:
> >> >> What you eventually see in hwmon is only a subset of all the features
> >> >> the IC provides. You may want to read this thread:
> >> >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg536509.html
> >> >
> >> > Okay, so the best thing to do is send out the entire patch-set at
> >> > once, CC'ing each of the maintainers on every patch so we can all see
> >> > how this thing fits together.
> >>
> >> Well, I am not even sure currently where to head with the MFD bits and
> >> its children subdevices currently....
> >>
> >> I would appreciate any direction. Yesterday, I was told on IRC, I
> >> would need to switch from i2c to platform drivers for the hwmon and
> >> gpio parts, but looking at some existing mfd driver code and their
> >> children drivers, I do not see it like that.
> >>
> >> I have already sent out the gpio driver yesterday which works fine on
> >> its own: http://www.spinics.net/lists/kernel/msg1655805.html
> >
> > This is going to need a lot of work.
> >
> > Did you run the patch through `./scripts/checkpatch.pl` before
> > submitting?
> 
> Of course, there has been zero errors and warnings. Eventually, I even
> ran the Lindent. Actual feedback is welcome for sure.

I barely have enough time to review my own subsystem, let alone
others. Linus will do a great job in this regard.

> >> Could you please guide me into the right direction what I need to
> >> change once we have standalone drivers, and they should be glued
> >> together? I thought adding an abstraction with the mfd layer would be
> >> sufficient, but apparently, that is not enough.
> >>
> >> Practically speaking, I am confused since if I needed to change the
> >> existing drivers, that means I could potentially break the interface
> >> for the existing users if the drivers stop working on their own, but
> >> then again, I am such a newbie that I would greatly appreciate some
> >> pointers.
> >
> > The MFD subsystem is quite simple to use. I'm taken aback that this is
> > your major stumbling block. Read though the mfd_add_device(s)() calls
> > to see what it expects. The rest is childs play.
> 
> Yeah, I have taken, but that does not still explain the consistency I
> mentioned above. Some children do not conform the "platform" driver
> suggestion I was told.
> 
> Also, what about the actual MFD code submitted? Anything to modify in
> there? Could you please comment on that, or is the direction of it
> good enough for me to submit it as a real patch at this stage?

Submit them all as I requested before and we will do a proper review.

Copy and pasting patches into conversation emails isn't the correct
method to use.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Simple MFD driver example
  2013-12-18 11:59                         ` Lee Jones
@ 2013-12-18 12:01                           ` Laszlo Papp
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Papp @ 2013-12-18 12:01 UTC (permalink / raw)
  To: Lee Jones; +Cc: sameo, LKML, Linus Walleij, Guenter Roeck

On Wed, Dec 18, 2013 at 11:59 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 18 Dec 2013, Laszlo Papp wrote:
>
>> On Wed, Dec 18, 2013 at 11:34 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> >> What you eventually see in hwmon is only a subset of all the features
>> >> >> the IC provides. You may want to read this thread:
>> >> >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg536509.html
>> >> >
>> >> > Okay, so the best thing to do is send out the entire patch-set at
>> >> > once, CC'ing each of the maintainers on every patch so we can all see
>> >> > how this thing fits together.
>> >>
>> >> Well, I am not even sure currently where to head with the MFD bits and
>> >> its children subdevices currently....
>> >>
>> >> I would appreciate any direction. Yesterday, I was told on IRC, I
>> >> would need to switch from i2c to platform drivers for the hwmon and
>> >> gpio parts, but looking at some existing mfd driver code and their
>> >> children drivers, I do not see it like that.
>> >>
>> >> I have already sent out the gpio driver yesterday which works fine on
>> >> its own: http://www.spinics.net/lists/kernel/msg1655805.html
>> >
>> > This is going to need a lot of work.
>> >
>> > Did you run the patch through `./scripts/checkpatch.pl` before
>> > submitting?
>>
>> Of course, there has been zero errors and warnings. Eventually, I even
>> ran the Lindent. Actual feedback is welcome for sure.
>
> I barely have enough time to review my own subsystem, let alone
> others. Linus will do a great job in this regard.
>
>> >> Could you please guide me into the right direction what I need to
>> >> change once we have standalone drivers, and they should be glued
>> >> together? I thought adding an abstraction with the mfd layer would be
>> >> sufficient, but apparently, that is not enough.
>> >>
>> >> Practically speaking, I am confused since if I needed to change the
>> >> existing drivers, that means I could potentially break the interface
>> >> for the existing users if the drivers stop working on their own, but
>> >> then again, I am such a newbie that I would greatly appreciate some
>> >> pointers.
>> >
>> > The MFD subsystem is quite simple to use. I'm taken aback that this is
>> > your major stumbling block. Read though the mfd_add_device(s)() calls
>> > to see what it expects. The rest is childs play.
>>
>> Yeah, I have taken, but that does not still explain the consistency I
>> mentioned above. Some children do not conform the "platform" driver
>> suggestion I was told.
>>
>> Also, what about the actual MFD code submitted? Anything to modify in
>> there? Could you please comment on that, or is the direction of it
>> good enough for me to submit it as a real patch at this stage?
>
> Submit them all as I requested before and we will do a proper review.
>
> Copy and pasting patches into conversation emails isn't the correct
> method to use.

Well, I am at the designing phase, and I am not even sure it is so far
OK. I am just trying to discuss the idea and direction, not the
implementation details yet. Not that I already wasted some time by not
asking before implementing stuff. See the previous hwmon patch for
details. I really do not want to waste more time if possible. It costs
a lot. :)

... but if you do not see design issues with it, then I will submit
the implementation for review and fixes, thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-12-18 12:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAOMwXhN5UfR=Y8V3i=A2zSCv7V9sRDd0Z+D1AQFahvRS-mV5Fw@mail.gmail.com>
2013-12-16  8:25 ` Simple MFD driver example Lee Jones
2013-12-16 14:00   ` Laszlo Papp
2013-12-16 15:05     ` Lee Jones
2013-12-16 16:39       ` Laszlo Papp
2013-12-16 16:48         ` Lee Jones
2013-12-17 17:09           ` Laszlo Papp
2013-12-18  9:16             ` Lee Jones
2013-12-18 10:42               ` Laszlo Papp
2013-12-18 11:12                 ` Lee Jones
2013-12-18 11:17                   ` Laszlo Papp
2013-12-18 11:34                     ` Lee Jones
2013-12-18 11:49                       ` Laszlo Papp
2013-12-18 11:59                         ` Lee Jones
2013-12-18 12:01                           ` Laszlo Papp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).