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: <e7f69aa3-20a7-4233-96c7-0fa5fe67bbdc@tuxon.dev>
Date: Fri, 23 Aug 2024 17:29:44 +0300
From: claudiu beznea <claudiu.beznea@...on.dev>
To: 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?



On 20.08.2024 15:17, Alexander Dahl wrote:
> 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?

Yes! This should be because system bus is clocked from MCK0 (as mentioned
in peripheral identifiers table) which is enabled by bootloader.

Here is a snapshot of reading the NVMEM on a SAMA7G5 with bootconfig and
thermal calibration packets:
https://www.linux4sam.org/bin/view/Linux4SAM/ThermalFaq

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