On 24/09/2024 at 17:52, Ryan Wanner wrote:
Hello Alex,
I think a possible solution is to put the DT binding ID for main rc oc
after PMC_MCK and then add 1 to all the other IDs that are not dependent
on PMC_MAIN, the IDs that are before the branch for the sama7g54.
One issue I see with this solution is with SoCs that do not want the
main rc os exported to the DT the driver array might be allocating too
much memory, this can be solved by removing the +1 that is in the clock
We're talking about a handful of bytes, we can surely afford that.
My $0.02. Regards,
Nicolas
drivers next to the device tree binding macro, since this macro is now
increased by 1 with this change.
Doing a quick test on the sam9x60 and sama7g54 I did not see any glaring
issues with this potential solution.
Best,
Ryan
On 9/19/24 05:39, Alexander Dahl wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hello Claudiu,
after being busy with other things, I'm back looking at this series.
As Nicolas pointed out [1], we need three clocks for the OTPC to work,
quote:
"for all the products, the main RC oscillator, the OTPC peripheral
clock and the MCKx clocks associated to OTP must be enabled."
I have a problem with making the main_rc_osc accessible for both
SAM9X60 and SAMA7G5 here, see below.
Am Wed, Aug 21, 2024 at 12:59:40PM +0200 schrieb Alexander Dahl:
SAM9X60 Datasheet (DS60001579G) Section "23.4 Product Dependencies"
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."
The code for enabling/disabling that clock is already present, it was
just not possible to hook into DT anymore, after at91 clk devicetree
binding rework back in 2018 for kernel v4.19.
Signed-off-by: Alexander Dahl <ada@xxxxxxxxxxx>
---
drivers/clk/at91/sam9x60.c | 3 ++-
include/dt-bindings/clock/at91.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
index e309cbf3cb9a..4d5ee20b8fc4 100644
--- a/drivers/clk/at91/sam9x60.c
+++ b/drivers/clk/at91/sam9x60.c
@@ -207,7 +207,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
if (IS_ERR(regmap))
return;
- sam9x60_pmc = pmc_data_allocate(PMC_PLLACK + 1,
+ sam9x60_pmc = pmc_data_allocate(PMC_MAIN_RC + 1,
nck(sam9x60_systemck),
nck(sam9x60_periphck),
nck(sam9x60_gck), 8);
@@ -218,6 +218,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
50000000);
if (IS_ERR(hw))
goto err_free;
+ sam9x60_pmc->chws[PMC_MAIN_RC] = hw;
hw = at91_clk_register_main_osc(regmap, "main_osc", mainxtal_name, NULL, 0);
if (IS_ERR(hw))
diff --git a/include/dt-bindings/clock/at91.h b/include/dt-bindings/clock/at91.h
index 3e3972a814c1..f957625cb3ac 100644
--- a/include/dt-bindings/clock/at91.h
+++ b/include/dt-bindings/clock/at91.h
@@ -25,6 +25,7 @@
#define PMC_PLLBCK 8
#define PMC_AUDIOPLLCK 9
#define PMC_AUDIOPINCK 10
+#define PMC_MAIN_RC 11
/* SAMA7G5 */
#define PMC_CPUPLL (PMC_MAIN + 1)
There are IDs defined in the devicetree bindings here, which are used
both in dts and in driver code as array indexes. In v1 of the patch
series I just added a new last element in the end of the generic list
and used that for SAM9X60.
For SAMA7G5 those IDs are branched of from PMC_MAIN in between, making
SAMA7G5 using a different last element, and different values after
PMC_MAIN.
Now we need a new ID for main rc osc, but not only for SAM9X60, but
also for SAMA7G5. I'm not sure what the implications would be, if the
new ID would be added in between before PMC_MAIN, so all values would
change? Adding it to the end of the lists would probably be safe, but
then you would need a diffently named variant for SAMA7G5's different
IDs. I find the current status somewhat unfortunate for future
extensions. How should this new ID be added here? What would be the
way forward?
Greets
Alex
[1] https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@xxxxxxxxxxxxx/T/#u
--
2.39.2
