Hi,
> -----Original Message-----
> From: Tony Lindgren <tony@xxxxxxxxxxx>
> Sent: Thursday, 19 September 2019 9:48 AM
> To: Ankur Tyagi <Ankur.Tyagi@xxxxxxxxxxxxx>
> Cc: t-kristo@xxxxxx; mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; linux-
> omap@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 1/2] clk: ti: am3: Update AM3 GPIO number as per
> datasheet
>
> * Ankur Tyagi <Ankur.Tyagi@xxxxxxxxxxxxx> [190918 21:44]:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Tony Lindgren <tony@xxxxxxxxxxx>
> > > Sent: Thursday, 19 September 2019 3:21 AM
> > > To: Ankur Tyagi <Ankur.Tyagi@xxxxxxxxxxxxx>
> > > Cc: t-kristo@xxxxxx; mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx;
> > > linux- omap@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH v3 1/2] clk: ti: am3: Update AM3 GPIO number as
> > > per datasheet
> > >
> > > Hi,
> > >
> > > * Ankur Tyagi <ankur.tyagi@xxxxxxxxxxxxx> [190917 23:49]:
> > > > Sitara technical reference manual numbers GPIO from 0 whereas in
> > > > code GPIO are numbered from 1
> > >
> > > If this is a cosmetic fix, please add it to the description.
> > > Then if there is also some other fix, that should be done separately
> >
> > This is a cosmetic fix only. Shall I send v4 with new description?
>
> Well if this is a cosmetic fix only, let's wait a bit so we can get rid of the
> "ti,hwmods" part. I'll post patches for that after -rc, so in few weeks. But
> yeah when reposting at that point, just add "no functional changes" to the
> patch to make it clear.
Sure, I'll wait
> > > > --- a/arch/arm/boot/dts/am33xx-l4.dtsi
> > > > +++ b/arch/arm/boot/dts/am33xx-l4.dtsi
> > > > @@ -129,7 +129,7 @@
> > > >
> > > > target-module@7000 {/* 0x44e07000, ap 14
> > > 20.0 */
> > > > compatible = "ti,sysc-omap2", "ti,sysc"; -ti,hwmods = "gpio1";
> > > > +ti,hwmods = "gpio0";
> > > > reg = <0x7000 0x4>,
> > > > <0x7010 0x4>,
> > > > <0x7114 0x4>;
> > >
> > > Let's simplify things a bit first :) We should be able to drop the
> "ti,hwmods"
> > > property for all gpio instances and the related platform data. I'll
> > > post a patch for that after -rc1.
> > >
> > > If there's some non-cosmetic fix here too, we should naturally apply
> > > a fix for that first.
> >
> > No, there is no other fix here
>
> OK thanks for confirming it.
>
> > > > -clocks = <&l4_wkup_clkctrl
> > > AM3_L4_WKUP_GPIO1_CLKCTRL 0>,
> > > > - <&l4_wkup_clkctrl
> > > AM3_L4_WKUP_GPIO1_CLKCTRL 18>;
> > > > +clocks = <&l4_wkup_clkctrl
> > > AM3_L4_WKUP_GPIO0_CLKCTRL 0>,
> > > > + <&l4_wkup_clkctrl
> > > AM3_L4_WKUP_GPIO0_CLKCTRL 18>;
> > >
> > > Not sure if this renumbering helps.. It might actually make it
> > > easier to introduce weird bugs if older dtb is used with a newer kernel.
> >
> > Actually I had some trouble with old version of kernel where I just
> > used
> > GPIO2 define to enable gpio2 clock but it enabled gpio1. That's why I
> > thought of fixing the numbering in code.
>
> Yeah fixing it makes things easier to follow :)
Thanks 😊
> > > > @@ -72,9 +72,9 @@ static const struct omap_clkctrl_reg_data
> > > am3_l4_per_clkctrl_regs[] __initconst
> > > > { AM3_RNG_CLKCTRL, NULL, CLKF_SW_SUP, "rng_fck" }, {
> > > > AM3_AES_CLKCTRL, NULL, CLKF_SW_SUP, "aes0_fck", "l3_clkdm" }, {
> > > > AM3_SHAM_CLKCTRL, NULL, CLKF_SW_SUP, "l3_gclk", "l3_clkdm"
> > > },
> > > > +{ AM3_GPIO1_CLKCTRL, am3_gpio1_bit_data, CLKF_SW_SUP,
> > > "l4ls_gclk" },
> > > > { AM3_GPIO2_CLKCTRL, am3_gpio2_bit_data, CLKF_SW_SUP,
> > > "l4ls_gclk" },
> > > > { AM3_GPIO3_CLKCTRL, am3_gpio3_bit_data, CLKF_SW_SUP,
> > > "l4ls_gclk" },
> > > > -{ AM3_GPIO4_CLKCTRL, am3_gpio4_bit_data, CLKF_SW_SUP,
> > > "l4ls_gclk" },
> > > > { AM3_TPCC_CLKCTRL, NULL, CLKF_SW_SUP, "l3_gclk", "l3_clkdm" },
> > > > { AM3_D_CAN0_CLKCTRL, NULL, CLKF_SW_SUP, "dcan0_fck" }, {
> > > > AM3_D_CAN1_CLKCTRL, NULL, CLKF_SW_SUP, "dcan1_fck" },
> > >
> > > So is this just renumbering, or do we have some other real bug too here?
> >
> > Just numbering to make things consistent with reference manual.
> > Let me know if it is worth doing then I'll send another patch with
> > updated description otherwise not 😊
>
> OK yeah let's wait a bit. Tero might have more comments too.
Sure, will wait for his comments and your patch
> Regards,
>
> Tony
Regards
Ankur
________________________________
This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof.
________________________________
