On Fri, Mar 22, 2013 at 11:23:32, Nori, Sekhar wrote:
> On 3/21/2013 1:31 PM, Philip, Avinash wrote:
> > On Wed, Mar 20, 2013 at 18:17:59, Peter Korsgaard wrote:
> >>>>>>> "Sekhar" == Sekhar Nori <nsekhar@xxxxxx> writes:
> >>
> >> Sekhar> On 3/20/2013 12:11 PM, Philip Avinash wrote:
> >> >> Add da850 EHRPWM & ECAP DT node.
> >> >> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP
> >> >> clock.
> >> >>
> >> >> Signed-off-by: Philip Avinash <avinashphilip@xxxxxx>
> >> >> ---
> >> >> Changes since v1:
> >> >> - Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are
> >> >> same with am33xx platform and da850 has no platform specific
> >> >> dependency.
> >>
> >> Sekhar> Which is fine, but I think the binding documentation still needs to be
> >> Sekhar> updated to document the ti,da850-ehrpwm binding. Looping Peter (it is
> >> Sekhar> always a good idea to CC folks who reviewed your patch last time
> >> Sekhar> around). Also, please Cc the DT folks and devicetree-discuss list too
> >> Sekhar> for their opinion.
> >>
> >> Yes, thanks for CC'ing me. I also think da850-* should be
> >> documented. See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for
> >> an similar (old) example.
> >
> > Peter,
> >
> > In this binding file also, only initial compatible field is updated. Later on many
> > compatible were added in driver. But not update back to binding doc.
>
> Probably someone forgot to keep updating the binding doc after a point.
>
> >
> > <Documentation/devicetree/bindings/gpio/8xxx_gpio.txt>
> > ---
> > followed by "fsl,mpc8349-gpio" for 83xx, "fsl,mpc8572-gpio" for 85xx and
> > "fsl,mpc8610-gpio" for 86xx.
> > ---
> > <drivers/gpio/gpio-mpc8xxx.c>
> > ---
> > static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
> > { .compatible = "fsl,mpc8349-gpio", },
> > { .compatible = "fsl,mpc8572-gpio", },
> > { .compatible = "fsl,mpc8610-gpio", },
> > { .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
> > { .compatible = "fsl,pq3-gpio", },
> > { .compatible = "fsl,qoriq-gpio", },
> > {}
> > };
> > ---
> >
> > Grant/Rob,
> > With respect peters explanation (below), what is your opinion on adding information to
> > binding doc for da850 support?
> >
> > Peter --> if the hardware block is identical the dts should simply list:
> > Peter --> compatible = "ti,da850-ecap", "ti,am33xx-ecap"
> > Peter --> And the driver only bind to ti,am33xx-ecap (unless there ever needs to
> > Peter --> be a da850 specific workaround.
> >
> > Or
> > Should I update both binding doc & the driver to use new compatible option "ti,da830-ecap"
> > (as da830 platform has initial IP support)?
> > Even with this, platforms da830, da850 and am33xx can reuse "ti,da830-ecap" compatible field.
>
> To me updating the driver to keep adding a .compatible even when not
> using it elsewhere in code is not required. Adding the new binding in
> .dts file is must since you may need to do something specific to da830
> at a later time (and the .dtb should be considered ROM'ed so you wont be
> able to change it then).
Thanks for the explanation.
I will add "ti,da850-ecap" for da850.dtsi along with "ti,am33xx-ecap" as
compatible fields.
> Adding documentation for the binding is also
> required to help anyone wanting to know more about the binding after
> reading the .dts file.
I will adding documentation part for "ti,da850-ecap".
Thanks
Avinash
>
> Thanks,
> Sekhar
>
��.n��������+%������w��{.n�����{����* jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
