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? > > --- 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 > > > -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. > > @@ -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 😊 > 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. ________________________________