RE: [PATCH 1/2] dt-bindings: clock: am33xx: Update GPIO number as per datasheet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Tero Kristo <t-kristo@xxxxxx>
> Sent: Thursday, 12 September 2019 7:36 PM
> To: Ankur Tyagi <Ankur.Tyagi@xxxxxxxxxxxxx>; mark.rutland@xxxxxxx;
> robh+dt@xxxxxxxxxx
> Cc: devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/2] dt-bindings: clock: am33xx: Update GPIO number as
> per datasheet
>
> On 12/09/2019 04:48, Ankur Tyagi wrote:
> > Sitara technical reference manual numbers GPIO from 0-3 whereas
> > in code GPIO are numbered from 1-4.
> >
>
> You are right that TRM states these are 0-3, but just to change the
> numbering at this point seems unnecessary churn for the minor
> inconvenience it causes. Also, I think we have similar indexing issues
> all over the place, should we fix them all? At least am4 has exact same
> indexing issue for gpios. They have been historically called 1-4 since
> introduction to linux kernel some 6 years already (as hwmod data). :)

I know it is a minor inconvenience once you figure it out 😊
One of our product is based upon am335x and I was writing a driver
which needed to read GPIO2 memory and was causing kernel panic as GPIO2 OCP clock
and fclk is not enabled by default. I looked inside am33xx-clocks.dtsi and found
"gpio0_dbclk_mux_ck" and my brain wired itself for numbering 0-3 and grabbed
AM3_GPIO2_CLKCTRL but clock was not getting enabled. Then after some debugging found
out numbering issue. And this can happen with anyone that's why thought of fixing it 😊

> Also, your patches cause bisect break points, try to apply this patch
> only to kernel and try to compile and see what happens.... and you don't
> touch the DT side for these either which causes another compile breakage.

I actually sent DT fix in separate mailing list, will resend everything in same list
to keep things simple. I will recheck the compilation issue, it worked in my setup

> So, from my side, NAK for both patches, I can't see why we should do
> such a small change and cause so many problems... not to mention the
> compile issues.
>
> Do you have some specific reason why you want these changed?
>
> -Tero

Regards
Ankur
>
> > Signed-off-by: Ankur Tyagi <ankur.tyagi@xxxxxxxxxxxxx>
> > ---
> >   include/dt-bindings/clock/am3.h | 16 ++++++++--------
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/dt-bindings/clock/am3.h b/include/dt-
> bindings/clock/am3.h
> > index 894951541276..980fdc05c3d0 100644
> > --- a/include/dt-bindings/clock/am3.h
> > +++ b/include/dt-bindings/clock/am3.h
> > @@ -41,9 +41,9 @@
> >   #define AM3_RNG_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0x90)
> >   #define AM3_AES_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0x94)
> >   #define AM3_SHAM_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xa0)
> > -#define AM3_GPIO2_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xac)
> > -#define AM3_GPIO3_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xb0)
> > -#define AM3_GPIO4_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xb4)
> > +#define AM3_GPIO1_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xac)
> > +#define AM3_GPIO2_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xb0)
> > +#define AM3_GPIO3_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xb4)
> >   #define AM3_TPCC_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xbc)
> >   #define AM3_D_CAN0_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xc0)
> >   #define AM3_D_CAN1_CLKCTRLAM3_L4_PER_CLKCTRL_INDEX(0xc4)
> > @@ -69,7 +69,7 @@
> >   #define AM3_L4_WKUP_CLKCTRL_OFFSET0x4
> >   #define AM3_L4_WKUP_CLKCTRL_INDEX(offset)((offset) -
> AM3_L4_WKUP_CLKCTRL_OFFSET)
> >   #define AM3_CONTROL_CLKCTRL
> AM3_L4_WKUP_CLKCTRL_INDEX(0x4)
> > -#define AM3_GPIO1_CLKCTRL
> AM3_L4_WKUP_CLKCTRL_INDEX(0x8)
> > +#define AM3_GPIO0_CLKCTRL
> AM3_L4_WKUP_CLKCTRL_INDEX(0x8)
> >   #define AM3_L4_WKUP_CLKCTRL
> AM3_L4_WKUP_CLKCTRL_INDEX(0xc)
> >   #define AM3_DEBUGSS_CLKCTRL
> AM3_L4_WKUP_CLKCTRL_INDEX(0x14)
> >   #define AM3_WKUP_M3_CLKCTRL
> AM3_L4_WKUP_CLKCTRL_INDEX(0xb0)
> > @@ -121,9 +121,9 @@
> >   #define AM3_L4LS_TIMER3_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0x84)
> >   #define AM3_L4LS_TIMER4_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0x88)
> >   #define AM3_L4LS_RNG_CLKCTRLAM3_L4LS_CLKCTRL_INDEX(0x90)
> > -#define AM3_L4LS_GPIO2_CLKCTRLAM3_L4LS_CLKCTRL_INDEX(0xac)
> > -#define AM3_L4LS_GPIO3_CLKCTRLAM3_L4LS_CLKCTRL_INDEX(0xb0)
> > -#define AM3_L4LS_GPIO4_CLKCTRLAM3_L4LS_CLKCTRL_INDEX(0xb4)
> > +#define AM3_L4LS_GPIO1_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0xac)
> > +#define AM3_L4LS_GPIO2_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0xb0)
> > +#define AM3_L4LS_GPIO3_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0xb4)
> >   #define AM3_L4LS_D_CAN0_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0xc0)
> >   #define AM3_L4LS_D_CAN1_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0xc4)
> >   #define AM3_L4LS_EPWMSS1_CLKCTRL
> AM3_L4LS_CLKCTRL_INDEX(0xcc)
> > @@ -184,7 +184,7 @@
> >
> >   /* l4_wkup clocks */
> >   #define AM3_L4_WKUP_CONTROL_CLKCTRL
> AM3_CLKCTRL_INDEX(0x4)
> > -#define AM3_L4_WKUP_GPIO1_CLKCTRLAM3_CLKCTRL_INDEX(0x8)
> > +#define AM3_L4_WKUP_GPIO0_CLKCTRLAM3_CLKCTRL_INDEX(0x8)
> >   #define AM3_L4_WKUP_L4_WKUP_CLKCTRL
> AM3_CLKCTRL_INDEX(0xc)
> >   #define AM3_L4_WKUP_UART1_CLKCTRLAM3_CLKCTRL_INDEX(0xb4)
> >   #define AM3_L4_WKUP_I2C1_CLKCTRLAM3_CLKCTRL_INDEX(0xb8)
> >
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-
> tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
________________________________
 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.
________________________________




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux