Re: [PATCH] ARM: dts: am33xx-l4: Add missing touchscreen clock properties

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi H.,

hns@goldelico.com wrote on Mon, 7 Mar 2022 20:19:48 +0100:

> Hi Miquel,
> some tiny typos...
> 
> > Am 07.03.2022 um 12:14 schrieb Miquel Raynal <miquel.raynal@bootlin.com>:
> > 
> > When adding support for TI magadc, the MFD driver (common to the  
> 
> "magadc"?

It's actually the name of the hardware block. It stands for 'magnetic
stripe reader and/or ADC', very much like the first ADC which has a
specific Touchscreen hardware feature as well. You can wire X lines to
a touchscreen, and TOTAL - X lines to the ADC, same applies to the
magnetic stripe reader.

I can s|magadc|Magnetic Stripe Reader/second ADC| to clarify.

> > touchscreen and the ADC) got updated to ease the insertion of a new DT
> > node for the ADC, with its own compatible, clocks, etc. Commit
> > 235a96e92c16 ("mfd: ti_am335x_tscadc: Don't search the tree for our
> > clock") removed one compatible specific information which was the clock
> > name, because the clock was looked up from scratch in the DT while this
> > hardware block was only fed by a single clock, already defined and
> > properly filled in the DT.
> > 
> > Problem is, this change was only validated with an am437x-based board,
> > where the clocks are effectively correctly defined and referenced. But
> > on am33xx, the ADC clock is also correctly defined but is not referenced
> > with a clock phandle as it out to be.  
> 
> maybe you mean "ought to be"?

I knew there was something wrong with it, but not what exactly :)

> > The touchscreen bindings clearly state that the clocks/clock-names
> > properties are mandatory, but they have been forgotten in one DTSI. This
> > was probably not noticed in the first place because of the clock
> > actually existing and the clk_get() call going through all the tree
> > anyway.
> > 
> > Add the missing clock phandles in the am33xx touchscreen description.  
> 
> Yes, makes touch on BeagleBoard Black with Chipsee 4"3 panel work again!
> 
> > 
> > Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> > Fixes: 235a96e92c16 ("mfd: ti_am335x_tscadc: Don't search the tree for our clock")  
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>

Thanks!
Miquèl

> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > 
> > Hello Nikolaus, as I told you I don't have the relevant hardware to
> > verify that this actually fixes your situation but I am rather
> > confident. Could you please give this a try?
> > Thanks! Miquel  
> 
> > 
> > arch/arm/boot/dts/am33xx-l4.dtsi | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
> > index c9629cb5ccd1..7da42a5b959c 100644
> > --- a/arch/arm/boot/dts/am33xx-l4.dtsi
> > +++ b/arch/arm/boot/dts/am33xx-l4.dtsi
> > @@ -263,6 +263,8 @@ tscadc: tscadc@0 {
> > 				compatible = "ti,am3359-tscadc";
> > 				reg = <0x0 0x1000>;
> > 				interrupts = <16>;
> > +				clocks = <&adc_tsc_fck>;
> > +				clock-names = "fck";
> > 				status = "disabled";
> > 				dmas = <&edma 53 0>, <&edma 57 0>;
> > 				dma-names = "fifo0", "fifo1";
> > -- 
> > 2.27.0
> >   
> 
> BR and thanks,
> Nikolaus
> 
>