Messages in this thread | | | Date | Tue, 16 Jun 2015 20:06:10 +0530 | From | Vaibhav Hiremath <> | Subject | Re: [PATCH 1/4] mfd: 88pm800: add device tree support |
| |
On Tuesday 16 June 2015 06:32 PM, Rob Herring wrote: > On Tue, Jun 16, 2015 at 2:52 AM, Vaibhav Hiremath > <vaibhav.hiremath@linaro.org> wrote: >> >> >> On Monday 01 June 2015 02:08 PM, Lee Jones wrote: >>> >>> On Sat, 30 May 2015, Vaibhav Hiremath wrote: >>> >> >> Thanks for your review. and sorry for delayed response. >> >>>> Add DT support to the 88pm800 driver along with below properties >>>> - marvell,88pm800-irq-write-clear : >>>> Idicates whether interrupt status is cleared by write >>>> >>>> Also, creates the DT binding text file, >>>> Documentation/devicetree/bindings/mfd/88pm800.txt >>>> >>>> Signed-off-by: Chao Xie <chao.xie@marvell.com> >>>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org> >>>> --- >>>> Documentation/devicetree/bindings/mfd/88pm800.txt | 57 >>>> +++++++++++++++++++++++ >>>> drivers/mfd/88pm800.c | 39 ++++++++++++++++ >>> >>> >>> These should be submitted separately. >>> >> >> >> Ok, will separate it in next version. >> >> >>>> 2 files changed, 96 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt >>>> b/Documentation/devicetree/bindings/mfd/88pm800.txt >>>> new file mode 100644 >>>> index 0000000..094951b >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt >>>> @@ -0,0 +1,57 @@ >>>> +* Marvell 88PM800 Power Management IC > > Might as well name this 88PM8xx from the start. >
Good point. Will include it in next version.
I am ready with V2, but luckily I did not sent next version, as I wanted to wait for Lee's response. :)
>>>> + >>>> +Required parent device properties: >>>> +- compatible : "marvell,88pm800" > > You might as well include 88pm805 and 88pm860 here since 805 support > is in the kernel and you will be submitting 860 support. They don't > have to be added with driver support. >
Ok.
>>>> +- reg : the I2C slave address for the 88pm800 chip >>>> +- interrupts : IRQ line for the 88pm800 chip >>>> +- interrupt-controller: describes the 88pm800 as an interrupt controller >>>> +- #interrupt-cells : should be 1. >>>> + - The cell is the 88pm800 local IRQ number >>>> + >>>> +Optional parent device properties: >>>> +- marvell,88pm800-irq-write-clr: indicates whether interrupt status is >>>> cleared by write >>> >>> >>> Drop the device name. These bindings should be as generic as possible. >>> >> >> OK, how about simply >> >> "mfd-irq_clr_on_write" > > No, mfd is a Linux name which doesn't belong in bindings, and > underscores are generally not used. >
Oops.
underscore is just typo. did not meant it.
> I think Lee meant marvell,irq-write-clr or irq-write-clr. >
will pick "marvell,irq-write-clr"
> This could also just be dropped altogether and set based on the > compatible strings with match data. >
It is configurable option, can be changed. so we still need get an input.
>>> Also describe what the absence of the property means. >>> >> >> Ok. >> >>>> +88pm800 consists of a large and varied group of sub-devices: >>> >>> >>> 3? >>> >> >> I have explicitly mentioned in note that more device list will follow. >> I just wanted to add entries as and when we add/enable driver support. >> >>>> +Device Supply Names Description >>>> +------ ------------ ----------- >>>> +88pm80x-onkey : : On key >>>> +88pm80x-rtc : : RTC >>>> +88pm80x : : Regulators >>> >>> >>> Surely regulators is 88pm80x-regulator, no? >>> >> >> did not understand what change is expected here. > > The node name for regulators node should be 88pm80x-regulator? But > these don't correspond to node names based on the example and the > example is correct IMO. So these are Linux driver names? If so, they > don't belong in the binding doc. What you need is to document the > sub-node compatible strings
Hmmm, Got it.
rtc and onkey are compatible strings only, I made mistake on regulator. I will change to "88pm80x-regulator".
Thanks, Vaibhav
| |