Dear Stephen,
On 2023/3/29 上午 11:25, Stephen Boyd wrote:
Quoting Jacky Huang (2023-03-28 20:13:11)
Dear Stephen,
On 2023/3/29 上午 10:46, Stephen Boyd wrote:
Quoting Jacky Huang (2023-03-28 19:39:36)
On 2023/3/29 上午 10:19, Stephen Boyd wrote:
What do you use the syscon for then? The clock driver must want to use
the syscon for something, implying that they are the same device.
The register lock mechanism is applied to protect many critical
registers from false written.
The register lock control register is one register in system controller.
Some registers of the clock controller are lock protected. Not only
clock controller, but other
IP such as RTC, PWM, ADC, etc, also have lock protected registers. All
these IP requires
syscon to access the lock/unlock control register in the system controller.
That's why we add a <&sys> to the clock controller.
Should we implement a ma35d1-sysctl driver to protect register_lock()
and register_unlock()
and export to those drivers? If yes, we can remove the <&sys> from
clock controller.
You can implement the lock and unlock in the hwspinlock framework. See
drivers/hwspinlock.
I may not explain clearly enough. The lock/unlock register of system
controller is more like
a kind of write protection for specific registers, rather than
preventing hetero-core CPU access.
In many different IP of ma35d1 contain write protected registers.
In fact, ma35d1 has a "hardware semaphore" IP, and we have implemented
the driver in drivers/hwspinlock.
Even the control register of "hardware semaphore" is also write protected.
What's the need to lock and unlock the registers? Is some other
processor also writing to the registers that we need to synchronize
against? Or is Linux the only entity reading and writing the registers?
I'm wondering if we should simply unlock the registers and never lock
them.
So, should we implement a system controller driver to provide
register_unlock() function?
Is it OK to have such a driver in drivers/mfd?
Or, just use syscon in device tree for those devices that have write
protect registers.
The hwspinlock framework doesn't require there to be another entity
accessing some resource. It's there to implement hardware locks. I don't
see why it can't be used here.
The current usage of register lock/unlock protect is as the following code:
static void ma35d1_unlock_regs(struct ma35d1_clk_pll *pll)
{
int ret;
do {
regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x59);
regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x16);
regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x88);
regmap_read(pll->regmap, REG_SYS_RLKTZNS, &ret);
} while (ret == 0);
}
static void ma35d1_lock_regs(struct ma35d1_clk_pll *pll)
{
regmap_write(pll->regmap, REG_SYS_RLKTZNS, 0x0);
}
And the following code is to unlock registers for write and then lock again.
ma35d1_unlock_regs(pll);
writel_relaxed(reg_ctl[0], pll->ctl0_base);
writel_relaxed(reg_ctl[1], pll->ctl1_base);
writel_relaxed(reg_ctl[2], pll->ctl2_base);
ma35d1_lock_regs(pll);
The above code is from the clk-ma35d1-pll.c from this patchset.
We just employ regmap mechansim for the access to REG_SYS_RLKTZNS register.
Is this implementation OK for you? Thank you.
Best regards,
Jacky Huang
