| Message ID | 1364197789-16783-4-git-send-email-avinashphilip@ti.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 3/25/2013 1:19 PM, Philip Avinash wrote: > Add da850 EHRPWM & ECAP DT node. > Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP > clock. This looks fine to me but I will wait for the bindings to get accepted before taking this one. Thanks, Sekhar
On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote: > On 3/25/2013 1:19 PM, Philip Avinash wrote: > > Add da850 EHRPWM & ECAP DT node. > > Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP > > clock. > > This looks fine to me but I will wait for the bindings to get accepted > before taking this one. Sekhar, Binding document got accepted in PWM tree [1]. Can you accept this patch? 1. https://gitorious.org/linux-pwm Thanks Avinash > > Thanks, > Sekhar >
On 4/8/2013 2:39 PM, Philip, Avinash wrote: > On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote: >> On 3/25/2013 1:19 PM, Philip Avinash wrote: >>> Add da850 EHRPWM & ECAP DT node. >>> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP >>> clock. >> >> This looks fine to me but I will wait for the bindings to get accepted >> before taking this one. > > Sekhar, > > Binding document got accepted in PWM tree [1]. > Can you accept this patch? Can you also add the pinmux definitions and resend just this patch? Sorry I did not notice those were missing earlier. Thanks, Sekhar
On Mon, Apr 08, 2013 at 18:39:57, Nori, Sekhar wrote: > > On 4/8/2013 2:39 PM, Philip, Avinash wrote: > > On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote: > >> On 3/25/2013 1:19 PM, Philip Avinash wrote: > >>> Add da850 EHRPWM & ECAP DT node. > >>> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP > >>> clock. > >> > >> This looks fine to me but I will wait for the bindings to get accepted > >> before taking this one. > > > > Sekhar, > > > > Binding document got accepted in PWM tree [1]. > > Can you accept this patch? > > Can you also add the pinmux definitions and resend just this patch? > Sorry I did not notice those were missing earlier. According to latest schematics, ECAP instance 2 being used for PWM backlight control. Should I add pin-mux only for ECAP2 or for all PWM instances? Thanks Avinash > > Thanks, > Sekhar >
On 4/9/2013 2:12 PM, Philip, Avinash wrote: > On Mon, Apr 08, 2013 at 18:39:57, Nori, Sekhar wrote: >> >> On 4/8/2013 2:39 PM, Philip, Avinash wrote: >>> On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote: >>>> On 3/25/2013 1:19 PM, Philip Avinash wrote: >>>>> Add da850 EHRPWM & ECAP DT node. >>>>> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP >>>>> clock. >>>> >>>> This looks fine to me but I will wait for the bindings to get accepted >>>> before taking this one. >>> >>> Sekhar, >>> >>> Binding document got accepted in PWM tree [1]. >>> Can you accept this patch? >> >> Can you also add the pinmux definitions and resend just this patch? >> Sorry I did not notice those were missing earlier. > > According to latest schematics, ECAP instance 2 being used for PWM backlight > control. Should I add pin-mux only for ECAP2 or for all PWM instances? I meant add definitions in .dtsi. Since there is only one pin a given functionality can be present on in DaVinci, it can be done in a board independent manner. See examples for other peripherals in existing da850.dtsi file. Thanks, Sekhar
On Tue, Apr 09, 2013 at 17:05:25, Nori, Sekhar wrote: > On 4/9/2013 2:12 PM, Philip, Avinash wrote: > > On Mon, Apr 08, 2013 at 18:39:57, Nori, Sekhar wrote: > >> > >> On 4/8/2013 2:39 PM, Philip, Avinash wrote: > >>> On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote: > >>>> On 3/25/2013 1:19 PM, Philip Avinash wrote: > >>>>> Add da850 EHRPWM & ECAP DT node. > >>>>> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP > >>>>> clock. > >>>> > >>>> This looks fine to me but I will wait for the bindings to get accepted > >>>> before taking this one. > >>> > >>> Sekhar, > >>> > >>> Binding document got accepted in PWM tree [1]. > >>> Can you accept this patch? > >> > >> Can you also add the pinmux definitions and resend just this patch? > >> Sorry I did not notice those were missing earlier. > > > > According to latest schematics, ECAP instance 2 being used for PWM backlight > > control. Should I add pin-mux only for ECAP2 or for all PWM instances? > > I meant add definitions in .dtsi. Since there is only one pin a given > functionality can be present on in DaVinci, it can be done in a board > independent manner. I think here the expectation would be that .dtsi should populate the complete pin-mux for SOC and board files should just be able to re-use it (add it as a phandler). Also as per the above description .dtsi file will end up contain majorly pin-mux info rather than the hardware data. Is it a good idea? On looking da850.dtsi file NAND pins were defined for 8-bit part. In case of NAND flash, the device might be sitting under different chip-select or may have 16 bit part on different boards. So pin-mux defined in soc.dtsi has to be split separately for CS, DATA, Address. So it is always challenging to create pin-mux info in .dtsi file. So more useful/meaningful way is to actually create pin-mux in board file rather in .dtsi file. > See examples for other peripherals in existing > da850.dtsi file. I have gone through .dtsi. But it didn't describe the complete pin-mux like I2C1, MMC1, etc. So the expectation here is only to add ECAP2 pin-mux. Is it correct? Thanks Avinash > > Thanks, > Sekhar >
On 4/10/2013 11:00 AM, Philip, Avinash wrote: > On Tue, Apr 09, 2013 at 17:05:25, Nori, Sekhar wrote: >> On 4/9/2013 2:12 PM, Philip, Avinash wrote: >>> On Mon, Apr 08, 2013 at 18:39:57, Nori, Sekhar wrote: >>>> >>>> On 4/8/2013 2:39 PM, Philip, Avinash wrote: >>>>> On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote: >>>>>> On 3/25/2013 1:19 PM, Philip Avinash wrote: >>>>>>> Add da850 EHRPWM & ECAP DT node. >>>>>>> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP >>>>>>> clock. >>>>>> >>>>>> This looks fine to me but I will wait for the bindings to get accepted >>>>>> before taking this one. >>>>> >>>>> Sekhar, >>>>> >>>>> Binding document got accepted in PWM tree [1]. >>>>> Can you accept this patch? >>>> >>>> Can you also add the pinmux definitions and resend just this patch? >>>> Sorry I did not notice those were missing earlier. >>> >>> According to latest schematics, ECAP instance 2 being used for PWM backlight >>> control. Should I add pin-mux only for ECAP2 or for all PWM instances? >> >> I meant add definitions in .dtsi. Since there is only one pin a given >> functionality can be present on in DaVinci, it can be done in a board >> independent manner. > > I think here the expectation would be that .dtsi should populate the complete > pin-mux for SOC and board files should just be able to re-use it (add it as a phandler). Yes, that's the idea. > Also as per the above description .dtsi file will end up contain majorly pin-mux info > rather than the hardware data. Is it a good idea? Pinmux is also hardware data, no? Thats why its present in DT. > On looking da850.dtsi file NAND pins were defined for 8-bit part. > In case of NAND flash, the device might be sitting under different chip-select or may > have 16 bit part on different boards. So pin-mux defined in soc.dtsi has to be split > separately for CS, DATA, Address. The idea is to define pin groups that most of the time can be reused by .dts file as-is and if there are any board specific extra pins needed then they can be handled directly in .dts files. But the common cases don't have to be repeated in all boards. In case of NAND, CS and the top 8-pins when using a 16-bit bus can be moved to a different group. So, I agree instead of nand_cs3_pins, we could have had nand_pins and moved cs definitions to another re-usable group. > So it is always challenging to create pin-mux info in .dtsi file. So more useful/meaningful > way is to actually create pin-mux in board file rather in .dtsi file. I don't see why it is so challenging. Repeating the same pinmux information over multiple .dts file (while making errors copying) will be challenging. And its not as if this is my original idea. imx (and I think some others) are doing it as well. See how pinmux is defined in imx53.dtsi and reused in a number of boards like evk, qsb, smd and so on. >> See examples for other peripherals in existing >> da850.dtsi file. > > I have gone through .dtsi. But it didn't describe the complete pin-mux like I2C1, MMC1, etc. pinmux should be added for whatever nodes are added since pimux is part of node. > So the expectation here is only to add ECAP2 pin-mux. Is it correct? No, please add pinmux information for all the IP nodes you are adding. I am not insisting that you add all IP nodes at the same time. You can add whatever you have tested. Thanks, Sekhar
On Wed, Apr 10, 2013 at 11:25:19, Nori, Sekhar wrote: > On 4/10/2013 11:00 AM, Philip, Avinash wrote: > > On Tue, Apr 09, 2013 at 17:05:25, Nori, Sekhar wrote: > >> On 4/9/2013 2:12 PM, Philip, Avinash wrote: > >>> On Mon, Apr 08, 2013 at 18:39:57, Nori, Sekhar wrote: > >>>> > >>>> On 4/8/2013 2:39 PM, Philip, Avinash wrote: > >>>>> On Tue, Apr 02, 2013 at 14:03:34, Nori, Sekhar wrote: > >>>>>> On 3/25/2013 1:19 PM, Philip Avinash wrote: > >>>>>>> Add da850 EHRPWM & ECAP DT node. > >>>>>>> Also adds OF_DEV_AUXDATA for EHRPWM & ECAP driver to use EHRPWM & ECAP > >>>>>>> clock. > >>>>>> > >>>>>> This looks fine to me but I will wait for the bindings to get accepted > >>>>>> before taking this one. > >>>>> > >>>>> Sekhar, > >>>>> > >>>>> Binding document got accepted in PWM tree [1]. > >>>>> Can you accept this patch? > >>>> > >>>> Can you also add the pinmux definitions and resend just this patch? > >>>> Sorry I did not notice those were missing earlier. > >>> > >>> According to latest schematics, ECAP instance 2 being used for PWM backlight > >>> control. Should I add pin-mux only for ECAP2 or for all PWM instances? > >> > >> I meant add definitions in .dtsi. Since there is only one pin a given > >> functionality can be present on in DaVinci, it can be done in a board > >> independent manner. > > > > I think here the expectation would be that .dtsi should populate the complete > > pin-mux for SOC and board files should just be able to re-use it (add it as a phandler). > > Yes, that's the idea. Ok > > > Also as per the above description .dtsi file will end up contain majorly pin-mux info > > rather than the hardware data. Is it a good idea? > > Pinmux is also hardware data, no? Thats why its present in DT. I understood. > > > On looking da850.dtsi file NAND pins were defined for 8-bit part. > > In case of NAND flash, the device might be sitting under different chip-select or may > > have 16 bit part on different boards. So pin-mux defined in soc.dtsi has to be split > > separately for CS, DATA, Address. > > The idea is to define pin groups that most of the time can be reused by > .dts file as-is and if there are any board specific extra pins needed > then they can be handled directly in .dts files. But the common cases > don't have to be repeated in all boards. In case of NAND, CS and the top > 8-pins when using a 16-bit bus can be moved to a different group. So, I > agree instead of nand_cs3_pins, we could have had nand_pins and moved cs > definitions to another re-usable group. Ok, thanks for the detailed explanation. > > > So it is always challenging to create pin-mux info in .dtsi file. So more useful/meaningful > > way is to actually create pin-mux in board file rather in .dtsi file. > > I don't see why it is so challenging. Repeating the same pinmux > information over multiple .dts file (while making errors copying) will > be challenging. And its not as if this is my original idea. imx (and I > think some others) are doing it as well. See how pinmux is defined in > imx53.dtsi and reused in a number of boards like evk, qsb, smd and so on. > > >> See examples for other peripherals in existing > >> da850.dtsi file. > > > > I have gone through .dtsi. But it didn't describe the complete pin-mux like I2C1, MMC1, etc. > > pinmux should be added for whatever nodes are added since pimux is part > of node. > > > So the expectation here is only to add ECAP2 pin-mux. Is it correct? > > No, please add pinmux information for all the IP nodes you are adding. I > am not insisting that you add all IP nodes at the same time. You can add > whatever you have tested. I actually tested EHRPWM1A and EHRPWM1B in older boards for back light support. But in latest schematics shows ECAP2 being used for backlight control and can't be tested as boards with this change is not accessible to me. So I will add tested pin-mux details. Thanks Avinash > > Thanks, > Sekhar >
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi index 3ec1bda..62fd2d4 100644 --- a/arch/arm/boot/dts/da850.dtsi +++ b/arch/arm/boot/dts/da850.dtsi @@ -107,6 +107,36 @@ reg = <0x21000 0x1000>; status = "disabled"; }; + ehrpwm0: ehrpwm@01f00000 { + compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm"; + #pwm-cells = <3>; + reg = <0x300000 0x2000>; + status = "disabled"; + }; + ehrpwm1: ehrpwm@01f02000 { + compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm"; + #pwm-cells = <3>; + reg = <0x302000 0x2000>; + status = "disabled"; + }; + ecap0: ecap@01f06000 { + compatible = "ti,da850-ecap", "ti,am33xx-ecap"; + #pwm-cells = <3>; + reg = <0x306000 0x80>; + status = "disabled"; + }; + ecap1: ecap@01f07000 { + compatible = "ti,da850-ecap", "ti,am33xx-ecap"; + #pwm-cells = <3>; + reg = <0x307000 0x80>; + status = "disabled"; + }; + ecap2: ecap@01f08000 { + compatible = "ti,da850-ecap", "ti,am33xx-ecap"; + #pwm-cells = <3>; + reg = <0x308000 0x80>; + status = "disabled"; + }; }; nand_cs3@62000000 { compatible = "ti,davinci-nand"; diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c index 6b7a0a2..89ee974 100644 --- a/arch/arm/mach-davinci/da8xx-dt.c +++ b/arch/arm/mach-davinci/da8xx-dt.c @@ -40,6 +40,11 @@ static void __init da8xx_init_irq(void) struct of_dev_auxdata da850_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL), OF_DEV_AUXDATA("ti,davinci-wdt", 0x01c21000, "watchdog", NULL), + OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f00000, "ehrpwm", NULL), + OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f02000, "ehrpwm", NULL), + OF_DEV_AUXDATA("ti,da850-ecap", 0x01f06000, "ecap", NULL), + OF_DEV_AUXDATA("ti,da850-ecap", 0x01f07000, "ecap", NULL), + OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap", NULL), {} };
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@ti.com> --- Changes since v1: - Reusing ti,am33xx<ecap/ehrpwm> as compatible field as both IP's are similar with am33xx platform and da850 has no platform specific dependency. arch/arm/boot/dts/da850.dtsi | 30 ++++++++++++++++++++++++++++++ arch/arm/mach-davinci/da8xx-dt.c | 5 +++++ 2 files changed, 35 insertions(+)