Message-ID: <20240820-stinging-altitude-d1186a067824@thorsis.com>
Date: Tue, 20 Aug 2024 14:20:21 +0200
From: Alexander Dahl <ada@...rsis.com>
To: claudiu beznea <claudiu.beznea@...on.dev>, linux-clk@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: get, prepare, enable a clock not in DT?

Hello,

Am Tue, Aug 20, 2024 at 02:17:58PM +0200 schrieb Alexander Dahl:
> Hello Claudiu,
> 
> Am Tue, Aug 20, 2024 at 02:54:59PM +0300 schrieb claudiu beznea:
> > Hi, Alexander,
> > 
> > On 16.08.2024 17:34, Alexander Dahl wrote:
> > > Hello everyone,
> > > 
> > > while further investigating timeout issues with the at91 otpc
> > > controller on sam9x60 [1] I came to the conclusion the main RC
> > > oscillator on that SoC must be enabled for that driver to work.
> > 
> > Not sure how that works (unless undocumented) as figure Figure 28-1. Clock
> > Generator Block Diagram from [1] states that main_rc_osc feeds only the mainck.
> 
> It can feed the main clock and you're right from Clock Generator POV.
> However it is not completely undocumented.  Section "23.4 Product
> Dependencies" of the SAM9X60 datasheet (DS60001579G) says:
> 
>     "The OTPC is clocked through the Power Management Controller (PMC).
>     The user must power on the main RC oscillator and enable the
>     peripheral clock of the OTPC prior to reading or writing the OTP
>     memory."
> 
> Apparently this also applies to reading, at least according to my
> tests on sam9x60-curiosity.
> 
> btw, the last public release of the atmel-software-package, source for
> the sam-ba applets, also enables that clock, although the reasoning
> was for writing. [1]

Sorry, forgot that reference, FWIW:

[1] https://github.com/atmelcorp/atmel-software-package/blob/master/drivers/nvm/otp/otpc.c#L99

Greets
Alex

> 
> > Also, Table 9-1. Peripheral Identifiers from [1] say that there is no clock
> > control for OTCP on the PMC side.
> > 
> > [1]
> > https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579.pdf
> 
> You're right from the datasheet POV.  Not sure if the datasheet is
> right here?  It's not complete in some register contents anyway, maybe
> some things are kept confidential, and OTPC is part of that?
> 
> Maybe someone can confirm my findings on sam9x60-curiosity, e.g.
> after I sent a patch series with what I consider fixes for this topic?
> 
> > > (Verified that by poking single bits in registers through devmem
> > > already.)
> > > 
> > > Fortunately the necessary clk is already registered from the SoC code
> > > in drivers/clk/at91/sam9x60.c [2] and I can see the clock in sysfs clk
> > > summary:
> > > 
> > >     root@...troKit:~ head -n4 /sys/kernel/debug/clk/clk_summary 
> > >                                      enable  prepare  protect                                duty  hardware                            connection
> > >        clock                          count    count    count        rate   accuracy phase  cycle    enable   consumer                         id
> > >     ---------------------------------------------------------------------------------------------------------------------------------------------
> > >      main_rc_osc                         0       0        0        12000000    50000000   0     50000      Y   deviceless                      no_connection_id         
> > > 
> > > That clock has no parent and is not found anywhere in devicetree, nor
> > > is it handled by the two clock-producers on that platform, so
> > > from within mchp_otpc_probe() I just tried this:
> > > 
> > >     otpc->clk = devm_clk_get_enabled(&pdev->dev, "main_rc_osc");
> > 
> > > 
> > > However that returns with -ENOENT, so I assume I can not reference the
> > > clock just by name?  Same result with this:
> > > 
> > >     otpc->clk = devm_clk_get_enabled(NULL, "main_rc_osc");
> > > 
> > > How do I get a pointer to that clk then to enable it?  Docs [3] where
> > 
> > To expose it though DT you may want to save its hw object to one array
> > entry in sam9x60_pmc, sam9x60_pmc->chws[] fits best for this atm.
> 
> Great to see I came to the same conclusion.  I have a proof-of-concept
> working meanwhile, will send a patch series later this week I guess.
> 
> Thanks for your support.
> 
> > Otherwise, you can try to register the main_rc_osc with CLK_IS_CRITICAL for
> > simple trials.
> 
> Don't think that is necessary anymore. :-)
> 
> By chance: I don't have a sama7g5 based board at hand for testing.
> The datasheet says the same as for sam9x60.
> Does the nvmem_microchip_otpc driver actually work without timeout on
> sama7g5?
> 
> Greets
> Alex
> 
> > 
> > Thank you,
> > Claudiu Beznea
> > 
> > > not as useful as I hoped for, neither was clk.h header docs. :-/
> > > 
> > > From what I understood from header docs reading 'device for clock
> > > "consumer"' I must pass the device from which I call that clk_get() as
> > > first parameter, so this would be the otpc device then, right?  What's
> > > that second parameter clock consumer id then?  Are these terms
> > > explained somewhere?
> > > 
> > > Greets
> > > Alex
> > > 
> > > [1] <20240813-payable-ecology-8a9e739704bb@...rsis.com>
> > > [2] https://elixir.bootlin.com/linux/v6.10.4/source/drivers/clk/at91/sam9x60.c#L217
> > > [3] https://kernel.org/doc/html/latest/driver-api/clk.html
> > > 
> > 
> 

Powered by blists - more mailing lists