linux-kernel - Re: get, prepare, enable a clock not in DT?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240820-grandpa-down-fec4231f971c@thorsis.com>
Date: Tue, 20 Aug 2024 14:17:58 +0200
From: Alexander Dahl <ada@...rsis.com>
To: claudiu beznea <claudiu.beznea@...on.dev>
Cc: 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 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]

> 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
> > 
>