* Re: [PATCH 00/10] Add support for a few Zc* extensions as well as Zcmop
From: Deepak Gupta @ 2024-04-10 21:34 UTC (permalink / raw)
To: Clément Léger
Cc: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Conor Dooley, Rob Herring, Krzysztof Kozlowski, Anup Patel,
Shuah Khan, Atish Patra, linux-doc, linux-riscv, linux-kernel,
devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <20240410091106.749233-1-cleger@rivosinc.com>
For the series:
Reviewed-by: Deepak Gupta <debug@rivosinc.com>
On Wed, Apr 10, 2024 at 2:13 AM Clément Léger <cleger@rivosinc.com> wrote:
>
> Add support for (yet again) more RVA23U64 missing extensions. Add
> support for Zcmop, Zca, Zcf, Zcd and Zcb extensions isa string parsing,
> hwprobe and kvm support. Zce, Zcmt and Zcmp extensions have been left
> out since they target microcontrollers/embedded CPUs and are not needed
> by RVA23U64
>
> This series is based on the Zimop one [1].
>
> Link: https://lore.kernel.org/linux-riscv/20240404103254.1752834-1-cleger@rivosinc.com/ [1]
>
> Clément Léger (10):
> dt-bindings: riscv: add Zca, Zcf, Zcd and Zcb ISA extension
> description
> riscv: add ISA parsing for Zca, Zcf, Zcd and Zcb
> riscv: hwprobe: export Zca, Zcf, Zcd and Zcb ISA extensions
> RISC-V: KVM: Allow Zca, Zcf, Zcd and Zcb extensions for Guest/VM
> KVM: riscv: selftests: Add some Zc* extensions to get-reg-list test
> dt-bindings: riscv: add Zcmop ISA extension description
> riscv: add ISA extension parsing for Zcmop
> riscv: hwprobe: export Zcmop ISA extension
> RISC-V: KVM: Allow Zcmop extension for Guest/VM
> KVM: riscv: selftests: Add Zcmop extension to get-reg-list test
>
> Documentation/arch/riscv/hwprobe.rst | 24 ++++++++++++
> .../devicetree/bindings/riscv/extensions.yaml | 37 +++++++++++++++++++
> arch/riscv/include/asm/hwcap.h | 5 +++
> arch/riscv/include/uapi/asm/hwprobe.h | 5 +++
> arch/riscv/include/uapi/asm/kvm.h | 5 +++
> arch/riscv/kernel/cpufeature.c | 5 +++
> arch/riscv/kernel/sys_hwprobe.c | 5 +++
> arch/riscv/kvm/vcpu_onereg.c | 10 +++++
> .../selftests/kvm/riscv/get-reg-list.c | 20 ++++++++++
> 9 files changed, 116 insertions(+)
>
> --
> 2.43.0
>
>
^ permalink raw reply
* Re: [PATCH v3 04/29] riscv: zicfilp / zicfiss in dt-bindings (extensions.yaml)
From: Deepak Gupta @ 2024-04-10 21:37 UTC (permalink / raw)
To: Rob Herring
Cc: paul.walmsley, rick.p.edgecombe, broonie, Szabolcs.Nagy,
kito.cheng, keescook, ajones, conor.dooley, cleger, atishp, alex,
bjorn, alexghiti, samuel.holland, conor, linux-doc, linux-riscv,
linux-kernel, devicetree, linux-mm, linux-arch, linux-kselftest,
corbet, palmer, aou, krzysztof.kozlowski+dt, oleg, akpm, arnd,
ebiederm, Liam.Howlett, vbabka, lstoakes, shuah, brauner,
andy.chiu, jerry.shih, hankuan.chen, greentime.hu, evan,
xiao.w.wang, charlie, apatel, mchitale, dbarboza, sameo,
shikemeng, willy, vincent.chen, guoren, samitolvanen,
songshuaishuai, gerg, heiko, bhe, jeeheng.sia, cyy, maskray,
ancientmodern4, mathis.salmen, cuiyunhui, bgray, mpe, baruch, alx,
david, catalin.marinas, revest, josh, shr, deller, omosnace,
ojeda, jhubbard
In-Reply-To: <20240410115806.GA4044117-robh@kernel.org>
On Wed, Apr 10, 2024 at 4:58 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Apr 03, 2024 at 04:34:52PM -0700, Deepak Gupta wrote:
> > Make an entry for cfi extensions in extensions.yaml.
> >
> > Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> > ---
> > .../devicetree/bindings/riscv/extensions.yaml | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > index 63d81dc895e5..45b87ad6cc1c 100644
> > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > @@ -317,6 +317,16 @@ properties:
> > The standard Zicboz extension for cache-block zeroing as ratified
> > in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> >
> > + - const: zicfilp
> > + description:
> > + The standard Zicfilp extension for enforcing forward edge control-flow
> > + integrity in commit 3a20dc9 of riscv-cfi and is in public review.
>
> Does in public review mean the commit sha is going to change?
>
Less likely. Next step after public review is to gather comments from
public review.
If something is really pressing and needs to be addressed, then yes
this will change.
Else this gets ratified as it is.
^ permalink raw reply
* Re: [PATCH 0/5] Add parsing for Zimop ISA extension
From: Deepak Gupta @ 2024-04-10 22:11 UTC (permalink / raw)
To: Clément Léger
Cc: Andrew Jones, Jonathan Corbet, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Anup Patel, Shuah Khan, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <ddc5555a-3ae8-42e5-a08a-ca5ceaf0bf28@rivosinc.com>
On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote:
>
>
>On 05/04/2024 19:33, Deepak Gupta wrote:
>> On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>>>
>>> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote:
>>>> The Zimop ISA extension was ratified recently. This series adds support
>>>> for parsing it from riscv,isa, hwprobe export and kvm support for
>>>> Guest/VM.
>>>
>>> I'm not sure we need this. Zimop by itself isn't useful, so I don't know
>>> if we need to advertise it at all. When an extension comes along that
>>> redefines some MOPs, then we'll advertise that extension, but the fact
>>> Zimop is used for that extension is really just an implementation detail.
>>
>> Only situation I see this can be useful is this:--
>>
>> An implementer, implemented Zimops in CPU solely for the purpose that they can
>> run mainline distro & packages on their hardware and don't want to leverage any
>> feature which are built on top of Zimop.
>
>Yes, the rationale was that some binaries using extensions that overload
>MOPs could still be run. With Zimop exposed, the loader could determine
>if the binary can be executed without potentially crashing. We could
>also let the program run anyway but the execution could potentially
>crash unexpectedly, which IMHO is not really good for the user
>experience nor for debugging. I already think that the segfaults which
>happens when executing binaries that need some missing extension are not
>so easy to debug, so better add more guards.
>
>>
>> As an example zicfilp and zicfiss are dependent on zimops. glibc can
>> do following
>>
>> 1) check elf header if binary was compiled with zicfiss and zicfilp,
>> if yes goto step 2, else goto step 6.
>> 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes
>> goto step 5. else goto step 3
>> 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4
>
>I think you meant step 5 rather than step 6.
No I did mean step 6 which is let the binary run. Step 5 is "issue the prctl" to
light up the feature, this should have been reached via step 2 if feature was
available.
Going to step 6 from step 3 basically means that underlying hardware only supports
zimops and thus this binary is safe to run on this hardware. But no need to issue
any prctl to kernel to enable this feature.
>
>Clément
>
>> 4) This binary won't be able to run successfully on this platform,
>> issue exit syscall. <-- termination
>> 5) issue prctl to enable shadow stack and landing pad for current task
>> <-- enable feature
>> 6) let the binary run <-- let the binary run because no harm can be done
^ permalink raw reply
* Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop
From: Conor Dooley @ 2024-04-10 22:16 UTC (permalink / raw)
To: Deepak Gupta
Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Rob Herring, Krzysztof Kozlowski,
Anup Patel, Shuah Khan, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <ZhcFeVYUQJmBAKuv@debug.ba.rivosinc.com>
[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]
On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
> On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
> > Add parsing for Zcmop ISA extension which was ratified in commit
> > b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
> >
> > Signed-off-by: Clément Léger <cleger@rivosinc.com>
> > ---
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/kernel/cpufeature.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index b7551bad341b..cff7660de268 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -86,6 +86,7 @@
> > #define RISCV_ISA_EXT_ZCB 77
> > #define RISCV_ISA_EXT_ZCD 78
> > #define RISCV_ISA_EXT_ZCF 79
> > +#define RISCV_ISA_EXT_ZCMOP 80
> >
> > #define RISCV_ISA_EXT_XLINUXENVCFG 127
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 09dee071274d..f1450cd7231e 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> > __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB),
> > __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD),
> > __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF),
> > + __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
>
> As per spec zcmop is dependent on zca. So perhaps below ?
>
> __RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP, RISCV_ISA_EXT_ZCA)
What's zicboz got to do with it, copy-pasto I guess?
If we're gonna imply stuff like this though I think we need some
comments explaining why it's okay.
> > __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> > __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > --
> > 2.43.0
> >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop
From: Conor Dooley @ 2024-04-10 22:27 UTC (permalink / raw)
To: Deepak Gupta
Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Rob Herring, Krzysztof Kozlowski,
Anup Patel, Shuah Khan, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <20240410-jawless-cavalry-a3eaf9c562a4@spud>
[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]
On Wed, Apr 10, 2024 at 11:16:11PM +0100, Conor Dooley wrote:
> On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
> > On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
> > > Add parsing for Zcmop ISA extension which was ratified in commit
> > > b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
> > >
> > > Signed-off-by: Clément Léger <cleger@rivosinc.com>
> > > ---
> > > arch/riscv/include/asm/hwcap.h | 1 +
> > > arch/riscv/kernel/cpufeature.c | 1 +
> > > 2 files changed, 2 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > index b7551bad341b..cff7660de268 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -86,6 +86,7 @@
> > > #define RISCV_ISA_EXT_ZCB 77
> > > #define RISCV_ISA_EXT_ZCD 78
> > > #define RISCV_ISA_EXT_ZCF 79
> > > +#define RISCV_ISA_EXT_ZCMOP 80
> > >
> > > #define RISCV_ISA_EXT_XLINUXENVCFG 127
> > >
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index 09dee071274d..f1450cd7231e 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> > > __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB),
> > > __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD),
> > > __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF),
> > > + __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
> >
> > As per spec zcmop is dependent on zca. So perhaps below ?
> >
> > __RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP, RISCV_ISA_EXT_ZCA)
>
> What's zicboz got to do with it, copy-pasto I guess?
> If we're gonna imply stuff like this though I think we need some
> comments explaining why it's okay.
Also, I'm inclined to call that out specifically in the binding, I've
not yet checked if dependencies actually work for elements of a string
array like the do for individual properties. I'll todo list that..
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop
From: Deepak Gupta @ 2024-04-10 22:32 UTC (permalink / raw)
To: Conor Dooley
Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Rob Herring, Krzysztof Kozlowski,
Anup Patel, Shuah Khan, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <20240410-judgingly-appease-5df493852b70@spud>
On Wed, Apr 10, 2024 at 11:27:16PM +0100, Conor Dooley wrote:
>On Wed, Apr 10, 2024 at 11:16:11PM +0100, Conor Dooley wrote:
>> On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
>> > On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
>> > > Add parsing for Zcmop ISA extension which was ratified in commit
>> > > b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
>> > >
>> > > Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> > > ---
>> > > arch/riscv/include/asm/hwcap.h | 1 +
>> > > arch/riscv/kernel/cpufeature.c | 1 +
>> > > 2 files changed, 2 insertions(+)
>> > >
>> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
>> > > index b7551bad341b..cff7660de268 100644
>> > > --- a/arch/riscv/include/asm/hwcap.h
>> > > +++ b/arch/riscv/include/asm/hwcap.h
>> > > @@ -86,6 +86,7 @@
>> > > #define RISCV_ISA_EXT_ZCB 77
>> > > #define RISCV_ISA_EXT_ZCD 78
>> > > #define RISCV_ISA_EXT_ZCF 79
>> > > +#define RISCV_ISA_EXT_ZCMOP 80
>> > >
>> > > #define RISCV_ISA_EXT_XLINUXENVCFG 127
>> > >
>> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> > > index 09dee071274d..f1450cd7231e 100644
>> > > --- a/arch/riscv/kernel/cpufeature.c
>> > > +++ b/arch/riscv/kernel/cpufeature.c
>> > > @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>> > > __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB),
>> > > __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD),
>> > > __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF),
>> > > + __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
>> >
>> > As per spec zcmop is dependent on zca. So perhaps below ?
>> >
>> > __RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP, RISCV_ISA_EXT_ZCA)
>>
>> What's zicboz got to do with it, copy-pasto I guess?
Yes, copy-pasta :-)
>> If we're gonna imply stuff like this though I think we need some
>> comments explaining why it's okay.
>
>Also, I'm inclined to call that out specifically in the binding, I've
>not yet checked if dependencies actually work for elements of a string
>array like the do for individual properties. I'll todo list that..
Earlier examples of specifying dependency on envcfg actually had functional
use case.
So you are right, I am not sure if its actually needed in this particular case.
And yes definitley, dependency should be mentioned in binding.
^ permalink raw reply
* Re: [PATCH v14 4/4] remoteproc: zynqmp: parse TCM from device tree
From: Tanmay Shah @ 2024-04-10 22:36 UTC (permalink / raw)
To: Mathieu Poirier
Cc: andersson, robh, krzysztof.kozlowski+dt, conor+dt, michal.simek,
ben.levinsky, linux-remoteproc, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <ZhbFfA7toAkUATfg@p14s>
On 4/10/24 11:59 AM, Mathieu Poirier wrote:
> On Mon, Apr 08, 2024 at 01:53:14PM -0700, Tanmay Shah wrote:
>> ZynqMP TCM information was fixed in driver. Now ZynqMP TCM information
>> is available in device-tree. Parse TCM information in driver
>> as per new bindings.
>>
>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>> ---
>>
>> Changes in v14:
>> - Add Versal platform support
>> - Add Versal-NET platform support
>> - Maintain backward compatibility for ZynqMP platform and use hardcode
>> TCM addresses
>> - Configure TCM based on xlnx,tcm-mode property for Versal
>> - Avoid TCM configuration if that property isn't available in DT
>>
>> drivers/remoteproc/xlnx_r5_remoteproc.c | 173 ++++++++++++++++++------
>> 1 file changed, 132 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
>> index 0f942440b4e2..504492f930ac 100644
>> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c
>> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
>> @@ -74,8 +74,8 @@ struct mbox_info {
>> };
>>
>> /*
>> - * Hardcoded TCM bank values. This will be removed once TCM bindings are
>> - * accepted for system-dt specifications and upstreamed in linux kernel
>> + * Hardcoded TCM bank values. This will stay in driver to maintain backward
>> + * compatibility with device-tree that does not have TCM information.
>> */
>> static const struct mem_bank_data zynqmp_tcm_banks_split[] = {
>> {0xffe00000UL, 0x0, 0x10000UL, PD_R5_0_ATCM, "atcm0"}, /* TCM 64KB each */
>> @@ -300,36 +300,6 @@ static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
>> dev_warn(dev, "failed to send message\n");
>> }
>>
>> -/*
>> - * zynqmp_r5_set_mode()
>> - *
>> - * set RPU cluster and TCM operation mode
>> - *
>> - * @r5_core: pointer to zynqmp_r5_core type object
>> - * @fw_reg_val: value expected by firmware to configure RPU cluster mode
>> - * @tcm_mode: value expected by fw to configure TCM mode (lockstep or split)
>> - *
>> - * Return: 0 for success and < 0 for failure
>> - */
>> -static int zynqmp_r5_set_mode(struct zynqmp_r5_core *r5_core,
>> - enum rpu_oper_mode fw_reg_val,
>> - enum rpu_tcm_comb tcm_mode)
>> -{
>> - int ret;
>> -
>> - ret = zynqmp_pm_set_rpu_mode(r5_core->pm_domain_id, fw_reg_val);
>> - if (ret < 0) {
>> - dev_err(r5_core->dev, "failed to set RPU mode\n");
>> - return ret;
>> - }
>> -
>> - ret = zynqmp_pm_set_tcm_config(r5_core->pm_domain_id, tcm_mode);
>> - if (ret < 0)
>> - dev_err(r5_core->dev, "failed to configure TCM\n");
>> -
>> - return ret;
>> -}
>> -
>> /*
>> * zynqmp_r5_rproc_start()
>> * @rproc: single R5 core's corresponding rproc instance
>> @@ -761,6 +731,103 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
>> return ERR_PTR(ret);
>> }
>>
>> +static int zynqmp_r5_get_tcm_node_from_dt(struct zynqmp_r5_cluster *cluster)
>> +{
>> + int i, j, tcm_bank_count, ret, tcm_pd_idx, pd_count;
>> + struct of_phandle_args out_args;
>> + struct zynqmp_r5_core *r5_core;
>> + struct platform_device *cpdev;
>> + struct mem_bank_data *tcm;
>> + struct device_node *np;
>> + struct resource *res;
>> + u64 abs_addr, size;
>> + struct device *dev;
>> +
>> + for (i = 0; i < cluster->core_count; i++) {
>> + r5_core = cluster->r5_cores[i];
>> + dev = r5_core->dev;
>> + np = r5_core->np;
>> +
>> + pd_count = of_count_phandle_with_args(np, "power-domains",
>> + "#power-domain-cells");
>> +
>> + if (pd_count <= 0) {
>> + dev_err(dev, "invalid power-domains property, %d\n", pd_count);
>> + return -EINVAL;
>> + }
>> +
>> + /* First entry in power-domains list is for r5 core, rest for TCM. */
>> + tcm_bank_count = pd_count - 1;
>> +
>> + if (tcm_bank_count <= 0) {
>> + dev_err(dev, "invalid TCM count %d\n", tcm_bank_count);
>> + return -EINVAL;
>> + }
>> +
>> + r5_core->tcm_banks = devm_kcalloc(dev, tcm_bank_count,
>> + sizeof(struct mem_bank_data *),
>> + GFP_KERNEL);
>> + if (!r5_core->tcm_banks)
>> + return -ENOMEM;
>> +
>> + r5_core->tcm_bank_count = tcm_bank_count;
>> + for (j = 0, tcm_pd_idx = 1; j < tcm_bank_count; j++, tcm_pd_idx++) {
>> + tcm = devm_kzalloc(dev, sizeof(struct mem_bank_data),
>> + GFP_KERNEL);
>> + if (!tcm)
>> + return -ENOMEM;
>> +
>> + r5_core->tcm_banks[j] = tcm;
>> +
>> + /* Get power-domains id of TCM. */
>> + ret = of_parse_phandle_with_args(np, "power-domains",
>> + "#power-domain-cells",
>> + tcm_pd_idx, &out_args);
>> + if (ret) {
>> + dev_err(r5_core->dev,
>> + "failed to get tcm %d pm domain, ret %d\n",
>> + tcm_pd_idx, ret);
>> + return ret;
>> + }
>> + tcm->pm_domain_id = out_args.args[0];
>> + of_node_put(out_args.np);
>> +
>> + /* Get TCM address without translation. */
>> + ret = of_property_read_reg(np, j, &abs_addr, &size);
>> + if (ret) {
>> + dev_err(dev, "failed to get reg property\n");
>> + return ret;
>> + }
>> +
>> + /*
>> + * Remote processor can address only 32 bits
>> + * so convert 64-bits into 32-bits. This will discard
>> + * any unwanted upper 32-bits.
>> + */
>> + tcm->da = (u32)abs_addr;
>> + tcm->size = (u32)size;
>> +
>> + cpdev = to_platform_device(dev);
>> + res = platform_get_resource(cpdev, IORESOURCE_MEM, j);
>> + if (!res) {
>> + dev_err(dev, "failed to get tcm resource\n");
>> + return -EINVAL;
>> + }
>> +
>> + tcm->addr = (u32)res->start;
>> + tcm->bank_name = (char *)res->name;
>> + res = devm_request_mem_region(dev, tcm->addr, tcm->size,
>> + tcm->bank_name);
>> + if (!res) {
>> + dev_err(dev, "failed to request tcm resource\n");
>> + return -EINVAL;
>> + }
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * zynqmp_r5_get_tcm_node()
>> * Ideally this function should parse tcm node and store information
>> @@ -839,9 +906,16 @@ static int zynqmp_r5_core_init(struct zynqmp_r5_cluster *cluster,
>> struct zynqmp_r5_core *r5_core;
>> int ret, i;
>>
>> - ret = zynqmp_r5_get_tcm_node(cluster);
>> - if (ret < 0) {
>> - dev_err(dev, "can't get tcm node, err %d\n", ret);
>> + r5_core = cluster->r5_cores[0];
>> +
>> + /* Maintain backward compatibility for zynqmp by using hardcode TCM address. */
>> + if (device_is_compatible(dev, "xlnx,zynqmp-r5fss"))
>
> The previous patch moved the definition of the R5FSS to the new bindings but
> this is forcing to use the old bindings - did I something?
Hi Mathieu,
We need to maintain backward compatibility for zynqmp device. So, using old bindings
for zynqmp. For new devices (Versal and Versal-NET) new bindings are enforced in driver.
It's not recommended to map two programming sequences to same device. So for
"xlnx,zynqmp-r5fss" device old bindings are used.
Device tree is matching with new bindings. But that's hardware description. I believe,
driver can still choose to use hardcode addresses to maintain backward compatibility.
The end result will be same.
Thanks,
Tanmay
>
>> + ret = zynqmp_r5_get_tcm_node(cluster);
>> + else
>> + ret = zynqmp_r5_get_tcm_node_from_dt(cluster);
>> +
>> + if (ret) {
>> + dev_err(dev, "can't get tcm, err %d\n", ret);
>> return ret;
>> }
>>
>> @@ -856,12 +930,18 @@ static int zynqmp_r5_core_init(struct zynqmp_r5_cluster *cluster,
>> return ret;
>> }
>>
>> - ret = zynqmp_r5_set_mode(r5_core, fw_reg_val, tcm_mode);
>> - if (ret) {
>> - dev_err(dev, "failed to set r5 cluster mode %d, err %d\n",
>> - cluster->mode, ret);
>> + ret = zynqmp_pm_set_rpu_mode(r5_core->pm_domain_id, fw_reg_val);
>> + if (ret < 0) {
>> + dev_err(r5_core->dev, "failed to set RPU mode\n");
>> return ret;
>> }
>> +
>> + if (device_is_compatible(dev, "xlnx,zynqmp-r5fss") ||
>> + of_find_property(dev_of_node(dev), "xlnx,tcm-mode", NULL)) {
>> + ret = zynqmp_pm_set_tcm_config(r5_core->pm_domain_id, tcm_mode);
>> + if (ret < 0)
>> + dev_err(r5_core->dev, "failed to configure TCM\n");
>> + }
>> }
>>
>> return 0;
>> @@ -906,16 +986,25 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
>> * fail driver probe if either of that is not set in dts.
>> */
>> if (cluster_mode == LOCKSTEP_MODE) {
>> - tcm_mode = PM_RPU_TCM_COMB;
>> fw_reg_val = PM_RPU_MODE_LOCKSTEP;
>> } else if (cluster_mode == SPLIT_MODE) {
>> - tcm_mode = PM_RPU_TCM_SPLIT;
>> fw_reg_val = PM_RPU_MODE_SPLIT;
>> } else {
>> dev_err(dev, "driver does not support cluster mode %d\n", cluster_mode);
>> return -EINVAL;
>> }
>>
>> + if (device_is_compatible(dev, "xlnx,zynqmp-r5fss")) {
>> + if (cluster_mode == LOCKSTEP_MODE)
>> + tcm_mode = PM_RPU_TCM_COMB;
>> + else
>> + tcm_mode = PM_RPU_TCM_SPLIT;
>> + } else if (of_find_property(dev_node, "xlnx,tcm-mode", NULL)) {
>> + ret = of_property_read_u32(dev_node, "xlnx,tcm-mode", (u32 *)&tcm_mode);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> /*
>> * Number of cores is decided by number of child nodes of
>> * r5f subsystem node in dts. If Split mode is used in dts
>> @@ -1100,6 +1189,8 @@ static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
>> /* Match table for OF platform binding */
>> static const struct of_device_id zynqmp_r5_remoteproc_match[] = {
>> { .compatible = "xlnx,zynqmp-r5fss", },
>> + { .compatible = "xlnx,versal-r5fss", },
>> + { .compatible = "xlnx,versal-net-r52fss", },
>> { /* end of list */ },
>> };
>> MODULE_DEVICE_TABLE(of, zynqmp_r5_remoteproc_match);
>> --
>> 2.25.1
>>
^ permalink raw reply
* RE: [PATCH v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension protocol
From: Peng Fan @ 2024-04-10 23:47 UTC (permalink / raw)
To: Rob Herring, Peng Fan (OSS)
Cc: Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Sudeep Holla,
Cristian Marussi, devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20240410171925.GA564262-robh@kernel.org>
> Subject: Re: [PATCH v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension
> protocol
>
> On Fri, Apr 05, 2024 at 08:39:24PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add i.MX SCMI Extension protocols bindings for:
> > - Battery Backed Secure Module(BBSM)
> > - MISC settings such as General Purpose Registers settings.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > .../devicetree/bindings/firmware/imx,scmi.yaml | 80
> ++++++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> > new file mode 100644
> > index 000000000000..7ee19a661d83
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) # Copyright 2024
> > +NXP %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Ffirmware%2Fimx%2Cscmi.yaml%23&data=05%7
> C02%7Cp
> >
> +eng.fan%40nxp.com%7C25c72418c9864a73924808dc59826288%7C686ea
> 1d3bc2b4c
> >
> +6fa92cd99c5c301635%7C0%7C0%7C638483663734828123%7CUnknown%
> 7CTWFpbGZsb
> >
> +3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D
> >
> +%7C0%7C%7C%7C&sdata=Y2uSGDorqR9HK8PO4AQDQ%2FaTv%2BETnulvU
> C8u9ktDoio%3
> > +D&reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-
> schemas%2Fcore.yaml%23&data=05%7C02%7Cpeng.fan%40nx
> >
> +p.com%7C25c72418c9864a73924808dc59826288%7C686ea1d3bc2b4c6fa
> 92cd99c5c
> >
> +301635%7C0%7C0%7C638483663734841350%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiM
> >
> +C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7
> C%7C%7
> >
> +C&sdata=h6lSgnPvR88CEBxmU%2B%2FCfCx9GHUrogWVxck38sIdhB4%3D&r
> eserved=0
> > +
> > +title: i.MX System Control and Management Interface(SCMI) Vendor
> > +Protocols Extension
> > +
> > +maintainers:
> > + - Peng Fan <peng.fan@nxp.com>
> > +
> > +allOf:
> > + - $ref: arm,scmi.yaml#
>
> This needs to be the other way around. Add a ref to this file in arm,scmi.yaml
> under an 'anyOf' entry.
ok, I will give a try.
>
> > +
> > +properties:
> > + protocol@81:
> > + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> > + unevaluatedProperties: false
> > + description:
> > + The BBM Protocol is for managing Battery Backed Secure Module
> (BBSM) RTC
> > + and the ON/OFF Key
> > +
> > + properties:
> > + reg:
> > + const: 0x81
> > +
> > + required:
> > + - reg
> > +
> > + protocol@84:
> > + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> > + unevaluatedProperties: false
> > + description:
> > + The MISC Protocol is for managing SoC Misc settings, such as
> > + GPR settings
> > +
> > + properties:
> > + reg:
> > + const: 0x84
> > +
> > + wakeup-sources:
> > + description:
> > + Each entry consists of 2 integers, represents the source and electric
> signal edge
> > + items:
> > + items:
> > + - description: the wakeup source
> > + - description: the wakeup electric signal edge
>
> No constraints on the entry values?
I will change the property name to imx,wakup-sources with constraints
minItems: 1
maxItems: 32.
>
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +
> > + required:
> > + - reg
> > +
> > +additionalProperties: false
>
> And then this can be true.
Fix in v3.
Thanks
Peng.
>
> Rob
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: rtc: convert trivial devices into dtschema
From: Andrew Jeffery @ 2024-04-10 23:55 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32, Krzysztof Kozlowski
In-Reply-To: <20240410-rtc_dtschema-v2-1-d32a11ab0745@gmail.com>
On Wed, 2024-04-10 at 17:55 +0200, Javier Carrasco wrote:
> These RTCs meet the requirements for a direct conversion into
> trivial-rtc:
>
> - orion-rtc
> - google,goldfish-rtc
> - maxim,ds1742
> - rtc-aspped
Nit: s/aspped/aspeed/, but otherwise it still looks fine for the Aspeed
bits and you can keep my R-b tag.
Andrew
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: sa8155p-adp: use correct gpio for SDHC2 CD
From: Volodymyr Babchuk @ 2024-04-10 21:57 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <Zhb53i8-pJhDMVZM@gerhold.net>
Hi Stephan,
Stephan Gerhold <stephan@gerhold.net> writes:
> On Wed, Apr 10, 2024 at 01:41:30PM +0000, Volodymyr Babchuk wrote:
>> Card Detect pin for SHDC2 on SA8155P-ADP is connected to GPIO4 of
>> PMM8155AU_1, not to internal TLMM. This change fixes two issues at
>> once: not working ethernet (because GPIO4 is used for MAC TX) and SD
>> detection.
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> ---
>> arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>> index 5e4287f8c8cd1..6b08ce246b78c 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>> @@ -384,7 +384,7 @@ &remoteproc_cdsp {
>> &sdhc_2 {
>> status = "okay";
>>
>> - cd-gpios = <&tlmm 4 GPIO_ACTIVE_LOW>;
>> + cd-gpios = <&pmm8155au_1_gpios 4 GPIO_ACTIVE_LOW>;
>
> Good catch!
Yeah... It took time to understand why ethernet is not working
sometimes. It appeared that there were race between SDHC and MAC
initialization.
>
>> pinctrl-names = "default", "sleep";
>> pinctrl-0 = <&sdc2_on>;
>> pinctrl-1 = <&sdc2_off>;
>
> These two pinctrl configure "gpio96" for "sd-cd-pins". Yet another wrong
> GPIO? Should probably drop that and add proper pinctrl for the actual
> GPIO in the PMIC. Seems like Qualcomm configured the PMIC GPIO with
> pull-up downstream [1] (not sure if this is the right file). It might be
> redundant if there is an external pull-up on the board, but only the
> schematic could tell that for sure.
If I only had schematic for this board... gpio96 puzzles me as well. I
can understand where wrong GPIO4 come from. But gpio96 origin is
completely unclear. I have user manual for the board, it mention
functions of some GPIOs, but there is nothing about gpio96. Anyways, I
removed it from the DTS (see diff below) and I see no issues with SD card.
> FWIW: Looking closer at the broken commit, the regulator voltage setup
> of &sdhc_2 looks suspicious too. Typically one would want a 1.8V capable
> regulator for the vqmmc-supply to properly use ultra high-speed modes,
> but &vreg_l13c_2p96 seems to be configured with 2.504V-2.960V at the
> moment. On downstream it seems to be 1.8V-2.96V [2] (again, not sure if
> this is the right file). I would recommend re-checking this too and
> testing if UHS cards are detected correctly (if you have the board).
This is actually a good catch. I changed the voltage range to 1.8V-2.96V and
now my card detects in UHS/SDR104 mode. Prior to this change it worked only
in HS mode.
> &vreg_l17a_2p96 has the same 2.504V-2.960V, but has 2.704V-2.960V
> downstream [3]. This is close at least, might be fine as-is (but I'm not
> convinced there is a good reason to differ there).
>
Well, I believe that downstream configuration is more correct, but I
can't prove it, so I'll leave it as is.
Diff for additional changes looks like this:
diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
index 6b08ce246b78c..b2a3496ff68ad 100644
--- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
@@ -283,7 +283,7 @@ vreg_l12c_1p808: ldo12 {
vreg_l13c_2p96: ldo13 {
regulator-name = "vreg_l13c_2p96";
- regulator-min-microvolt = <2504000>;
+ regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <2960000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
@@ -386,8 +386,8 @@ &sdhc_2 {
cd-gpios = <&pmm8155au_1_gpios 4 GPIO_ACTIVE_LOW>;
pinctrl-names = "default", "sleep";
- pinctrl-0 = <&sdc2_on>;
- pinctrl-1 = <&sdc2_off>;
+ pinctrl-0 = <&sdc2_on &pmm8155au_1_sdc2_on>;
+ pinctrl-1 = <&sdc2_off &pmm8155au_1_sdc2_off>;
vqmmc-supply = <&vreg_l13c_2p96>; /* IO line power */
vmmc-supply = <&vreg_l17a_2p96>; /* Card power line */
bus-width = <4>;
@@ -505,13 +505,6 @@ data-pins {
bias-pull-up; /* pull up */
drive-strength = <16>; /* 16 MA */
};
-
- sd-cd-pins {
- pins = "gpio96";
- function = "gpio";
- bias-pull-up; /* pull up */
- drive-strength = <2>; /* 2 MA */
- };
};
sdc2_off: sdc2-off-state {
@@ -532,13 +525,6 @@ data-pins {
bias-pull-up; /* pull up */
drive-strength = <2>; /* 2 MA */
};
-
- sd-cd-pins {
- pins = "gpio96";
- function = "gpio";
- bias-pull-up; /* pull up */
- drive-strength = <2>; /* 2 MA */
- };
};
usb2phy_ac_en1_default: usb2phy-ac-en1-default-state {
@@ -604,3 +590,25 @@ phy-reset-pins {
};
};
};
+
+&pmm8155au_1_gpios {
+ pmm8155au_1_sdc2_on: pmm8155au_1-sdc2-on-state {
+ sd-cd-pin {
+ pins = "gpio4";
+ function = "normal";
+ input-enable;
+ bias-pull-up;
+ power-source = <0>;
+ };
+ };
+
+ pmm8155au_1_sdc2_off: pmm8155au_1-sdc2-off-state {
+ sd-cd-pin {
+ pins = "gpio4";
+ function = "normal";
+ input-enable;
+ bias-pull-up;
+ power-source = <0>;
+ };
+ };
+};
I am planning to send v2 tomorrow.
--
WBR, Volodymyr
^ permalink raw reply related
* [PATCH] riscv: dts: sophgo: add reserved memory node for CV1800B
From: Inochi Amaoto @ 2024-04-11 0:45 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen Wang,
Inochi Amaoto, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Jisheng Zhang, Guo Ren, Drew Fustini
Cc: devicetree, linux-riscv, linux-kernel
The original dts of CV1800B has a weird memory length as it
contains reserved memory for coprocessor. Make this area a
separate node so it can get the real memory length.
Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
---
arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts | 12 +++++++++---
arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 5 +++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..0b9128513357 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
@@ -23,9 +23,15 @@ chosen {
stdout-path = "serial0:115200n8";
};
- memory@80000000 {
- device_type = "memory";
- reg = <0x80000000 0x3f40000>;
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ coprocessor_rtos: region@83f40000 {
+ reg = <0x83f40000 0xc0000>;
+ no-map;
+ };
};
};
diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index 165e9e320a8c..17ce8aa415a8 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
@@ -7,6 +7,11 @@
/ {
compatible = "sophgo,cv1800b";
+
+ memory@80000000 {
+ device_type = "memory";
+ reg = <0x80000000 0x4000000>;
+ };
};
&plic {
--
2.44.0
^ permalink raw reply related
* RE: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set additionalProperties to true
From: Peng Fan @ 2024-04-11 1:50 UTC (permalink / raw)
To: Cristian Marussi, Rob Herring
Cc: Krzysztof Kozlowski, Peng Fan (OSS), Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Sudeep Holla, devicetree@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <ZhVXIAHWQv3mHBgP@pluto>
Hi Rob, Cristian,
> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set
> additionalProperties to true
>
> On Tue, Apr 09, 2024 at 09:09:46AM -0500, Rob Herring wrote:
> > On Tue, Apr 9, 2024 at 7:01 AM Cristian Marussi
> > <cristian.marussi@arm.com> wrote:
> > >
> > > On Tue, Apr 09, 2024 at 09:25:10AM +0000, Peng Fan wrote:
> > > > Hi Krzysztof,
> > > >
> > > > > Subject: RE: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set
> > > > > additionalProperties to true
> > > > >
> > > > > > Subject: Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi:
> > > > > > set additionalProperties to true
> > > > > >
> > > > > > On 08/04/2024 08:08, Peng Fan wrote:
> > > > > > >> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware:
> > > > > > >> arm,scmi: set additionalProperties to true
> > > > > > >>
> > > > > > >> On 08/04/2024 01:50, Peng Fan wrote:
> > > > > > >>>> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware:
> > > > > > >>>> arm,scmi: set additionalProperties to true
> > > > > > >>>>
> > > > > > >>>> On 07/04/2024 12:04, Peng Fan wrote:
> > > > > > >>>>>> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi:
> > > > > > >>>>>> set additionalProperties to true
> > > > > > >>>>>>
> > > > > > >>>>>> On 07/04/2024 02:37, Peng Fan wrote:
> > > > > > >>>>>>>> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware:
> arm,scmi:
> > > > > > >>>>>>>> set additionalProperties to true
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> > > > > > >>>>>>>>> From: Peng Fan <peng.fan@nxp.com>
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> When adding vendor extension protocols, there is
> > > > > > >>>>>>>>> dt-schema
> > > > > > >> warning:
> > > > > > >>>>>>>>> "
> > > > > > >>>>>>>>> imx,scmi.example.dtb: scmi: 'protocol@81',
> > > > > > >>>>>>>>> 'protocol@84' do not match any of the regexes: 'pinctrl-
> [0-9]+'
> > > > > > >>>>>>>>> "
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> Set additionalProperties to true to address the issue.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> I do not see anything addressed here, except making
> > > > > > >>>>>>>> the binding accepting anything anywhere...
> > > > > > >>>>>>>
> > > > > > >>>>>>> I not wanna add vendor protocols in arm,scmi.yaml, so
> > > > > > >>>>>>> will introduce a new yaml imx.scmi.yaml which add i.MX
> > > > > > >>>>>>> SCMI protocol
> > > > > > >> extension.
> > > > > > >>>>>>>
> > > > > > >>>>>>> With additionalProperties set to false, I not know
> > > > > > >>>>>>> how, please
> > > > > > suggest.
> > > > > > >>>>>>
> > > > > > >>>>>> First of all, you cannot affect negatively existing
> > > > > > >>>>>> devices (their
> > > > > > >>>>>> bindings) and your patch does exactly that. This should
> > > > > > >>>>>> make you thing what is the correct approach...
> > > > > > >>>>>>
> > > > > > >>>>>> Rob gave you the comment about missing compatible - you
> > > > > > >>>>>> still did not address that.
> > > > > > >>>>>
> > > > > > >>>>> I added the compatible in patch 2/6 in the examples
> > > > > > >>>>> "compatible =
> > > > > > >>>> "arm,scmi";"
> > > > > > >>>>
> > > > > > >>>> So you claim that your vendor extensions are the same or
> > > > > > >>>> fully compatible with arm,scmi and you add nothing... Are
> > > > > > >>>> your extensions/protocol valid for arm,scmi?
> > > > > > >>>
> > > > > > >>> Yes. They are valid for arm,scmi.
> > > > > > >>>
> > > > > > >>> If yes, why is this in separate binding. If no, why you
> > > > > > >>> use someone
> > > > > > >>>> else's compatible?
> > > > > > >>>
> > > > > > >>> Per SCMI Spec
> > > > > > >>> 0x80-0xFF: Reserved for vendor or platform-specific
> > > > > > >>> extensions to this interface
> > > > > > >>>
> > > > > > >>> i.MX use 0x81 for BBM, 0x84 for MISC. But other vendors
> > > > > > >>> will use the id for their own protocol.
> > > > > > >>
> > > > > > >> So how are they valid for arm,scmi? I don't understand.
> > > > > > >
> > > > > > > arm,scmi is a firmware compatible string. The protocol node is a
> sub-node.
> > > > > > > I think the arm,scmi is that saying the firmware following
> > > > > > > SCMI spec to implement the protocols.
> > > > > > >
> > > > > > > For vendor reserved ID, firmware also follow the SCMI spec
> > > > > > > to implement their own usage, so from firmware level, it is
> > > > > > > ARM SCMI spec
> > > > > > compatible.
> > > > > >
> > > > > > That's not the point. It is obvious that your firmware is
> > > > > > compatible with arm,scmi, but what you try to say in this and
> > > > > > revised patch is that every arm,scmi is compatible with your
> > > > > > implementation. What you are saying is that 0x84 is MISC
> > > > > > protocol for every firmware, Qualcomm, NXP, Samsung, TI, Mediatek
> etc.
> > > > > >
> > > > > > I claim it is not true. 0x84 is not MISC for Qualcomm, for example.
> > > > >
> > > > > You are right. I am lost now on how to add vendor ID support,
> > > > > using arm,scmi.yaml or adding a new imx,scmi.yaml or else.
> > > >
> > >
> > > Hi Peng,
> > >
> > > I dont think in the following you will find the solution to the
> > > problem, it is just to recap the situation and constraints around
> > > vendor protocol bindings.
> > >
> > > Describing SCMI vendors protocols is tricky because while on one
> > > side the protocol node has to be rooted under the main scmi fw DT
> > > node (like all the standard protocols) and be 'derived' from the
> > > arm,scmi.yaml protocol-node definition, the optional additional
> > > properties of the a specific vendor protocol nodes can be customized
> > > by each single vendor, and since, as you said, you can have multiple
> > > protocols from different vendors sharing the same protocol number,
> > > you could have multiple disjoint sets of valid properties allowed
> > > under that same protocol node number; so on one side you have to
> > > stick to some basic protocol-node defs and be rooted under the SCMI
> > > node, while on the other side you will have multiple possibly
> > > allowed sets of additional properties to check against, so IOW you cannot
> anyway just set additionalProperties to false since that will result in no checks
> at all.
> > >
> > > As a consequence, at runtime, in the final DTB shipped with a
> > > specific platform you should have only one of the possible vendor
> > > nodes for each of these overlapping protocols, and the SCMI core at
> > > probe time will pick the proper protocol implementation based on the
> > > vendor/sub_vendor IDs gathered from the running SCMI fw platform at
> > > init: this way you can just build the usual "all-inclusive"
> > > defconfig without worrying about vendor protocol clashes since the
> > > SCMI core can pick the right protocol implementation, you should
> > > just had taken care to provide the proper DTB for your protocol; BUT
> > > this also means that it is not possible to add multiple DT bindings
> > > based on a 'if vendor' condition since the vendor itself is NOT
> > > defined and not needed in the bindings since it is discoverable at runtime.
> > >
> > > So, after all of this blabbing of mine about this, I am wondering if
> > > it is not possible that the solution is to handle each and every
> > > vendor protocol node that appears with a block of addtional
> > > properties that are picked via a oneOf statement from some external
> > > vendor specific yaml.
> > > (...in a similar way to how pinctrl additional properties are
> > > added...)
> > >
> > >
> > > NOTE THAT the following is just an example of what I mean, it is
> > > certainly wrong, incomplete annd maybe just not acceptable (and
> > > could cause DT maintainers eyes to bleed :P)...
> > >
> > > ...so it is just fr the sake of explaining what I mean...
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > index e9d3f043c4ed..3c38a1e3ffed 100644
> > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > @@ -278,6 +278,22 @@ properties:
> > > required:
> > > - reg
> > >
> > > + protocol@81:
> > > + $ref: '#/$defs/protocol-node'
> > > + unevaluatedProperties: false
> > > +
> > > + properties:
> > > + reg:
> > > + const: 0x81
> > > +
> > > + patternProperties:
> > > + '$':
> > > + type: object
> >
> > Did you mean to have child nodes under the protocol node rather than in it?
>
> ... nope ... it is just as bad as my yaml-fu is :P ... but not sure if vendors has
> also this needs or plain props will suffice...
>
> >
> > > + oneOf:
> > > + - $ref: /schemas/vendor-A/scmi-protos.yaml#
> > > + - $ref: /schemas/vendor-B/protos.yaml#
> >
> > Moved up one level, this would work, but it would have to be an
> > 'anyOf' because it is possible that 2 vendors have the exact same set
> > of properties.
> >
I try this:
protocol@84:
anyOf:
- $ref: /schemas/firmware/imx,scmi.yaml#
- $ref: /schemas/firmware/vendor-B,scmi.yaml#
but unevaluatedProperties could not be set to false, otherwise the
example check will report
reg is unevaluated, fsl,wakeup-sources is unevaluated.
The imx,scmi.yaml is as below:
properties:
protocol@84:
$ref: 'arm,scmi.yaml#/$defs/protocol-node'
unevaluatedProperties: false
description:
The MISC Protocol is for managing SoC Misc settings, such as GPR settings
properties:
reg:
const: 0x84
fsl,wakeup-sources:
description:
Each entry consists of 2 integers, represents the source and electric signal edge
items:
items:
- description: the wakeup source
- description: the wakeup electric signal edge
minItems: 1
maxItems: 32
$ref: /schemas/types.yaml#/definitions/uint32-matrix
required:
- reg
additionalProperties: true
Is the upper ok?
Thanks,
Peng.
>
> ok
>
> > I can think of 2 other ways to structure this.
> >
> > First, is a specific vendor protocol discoverable? Not that is 0x81
> > protocol present, but that 0x81 is vendor Foo's extra special
> > value-add protocol? If not, I think we should require a compatible
> > string on vendor protocols. Then the base SCMI schema can require just
> > that, and each vendor protocol defines its node with a $ref to
> > '#/$defs/protocol-node'.
>
> Basically yes it is discoverable, since at runtime the SCMI core, early on,
> normally discovers the vendor_id/sub_vendor_id by querying the platform
> via Base protocol and then later only loads/initializes (by closest match) the
> vendor protocols that are present in the DT AND that has been 'tagged' at
> compile time with the same vendor_id/sub_vendor_id tuple (in the vendor
> module code, struct scmi_protocol)
>
> Of course you should take care to put the proper protocol@81 node in your
> vendor_A DTB for the vendor_A SCMI driver to make use of the additional
> vendor_A properties that you have defined under your node as referred in
> your vendor-protos.yaml...if you botch that up I will load a protocol and call
> your vendor_A driver with a vendor_X DT node.
>
> DT is currrently vendor-agnostic.
>
> >
> > The 2nd way is just a variation of the oneOf above, but do we do 1
> > file per vendor protocol or 1 file per vendor. Either should be
> > doable, just a matter of where 'protocol@81', etc. are defined.
> >
>
> Oh, yes mine was just an ill example...one file per vendor will do just
> fine: the important thing is that the list and the yaml itself can be extended as
> new vendors appears (in a backward compatble way of course)
>
> Thanks,
> Cristian
^ permalink raw reply
* Re: [PATCH 1/2] media: dt-bindings: i2c: add Giantec GT97xx VCM driver
From: Zhi Mao (毛智) @ 2024-04-11 2:04 UTC (permalink / raw)
To: conor@kernel.org
Cc: heiko@sntech.de, linux-kernel@vger.kernel.org,
laurent.pinchart+renesas@ideasonboard.com, yunkec@chromium.org,
linux-mediatek@lists.infradead.org, linux-media@vger.kernel.org,
hdegoede@redhat.com, bingbu.cao@intel.com,
paul.elder@ideasonboard.com, devicetree@vger.kernel.org,
andy.shevchenko@gmail.com, mchehab@kernel.org,
Shengnan Wang (王圣男),
Yaya Chang (張雅清), p.zabel@pengutronix.de,
alain.volmat@foss.st.com, conor+dt@kernel.org,
sakari.ailus@linux.intel.com, robh@kernel.org,
hverkuil-cisco@xs4all.nl, tomi.valkeinen@ideasonboard.com,
linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
mehdi.djait@bootlin.com, krzk+dt@kernel.org,
angelogioacchino.delregno@collabora.com, 10572168@qq.com
In-Reply-To: <20240410-rice-fringe-4ae992217a2f@spud>
Hi Conor,
Thanks for your review.
On Wed, 2024-04-10 at 12:27 +0100, Conor Dooley wrote:
> >
> >
> Hey,
>
> On Wed, Apr 10, 2024 at 06:40:01PM +0800, Zhi Mao wrote:
> > b/Documentation/devicetree/bindings/media/i2c/giantec,gt97xx.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (c) 2020 MediaTek Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/giantec,gt97xx.yaml#
>
> Filename patching compatible please.
>
>
Sorry, I don't catch this point.
Can you explain more details?
> >
> >
> > +
> > + giantec,aac-mode:
> > + description:
> > + Indication of AAC mode select.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum:
> > + - 1 # AAC2 mode(operation time# 0.48 x Tvib)
> > + - 2 # AAC3 mode(operation time# 0.70 x Tvib)
> > + - 3 # AAC4 mode(operation time# 0.75 x Tvib)
> > + - 5 # AAC8 mode(operation time# 1.13 x Tvib)
>
> I dislike these enum based properties and I would rather this either
> be
> the values themselves (0.48, 0.70 etc).
>
> > +
> > + giantec,aac-timing:
> > + description:
> > + Number of AAC Timing count that controlled by one 6-bit
> > period of
> > + vibration register AACT[5:0], the unit of which is 100 us.
>
> Then the property should be in a standard unit of time, not "random"
> hex
> numbers that correspond to register values.
>
> >
> > + giantec,clock-presc:
> > + description:
> > + Indication of VCM internal clock dividing rate select, as
> > one multiple
> > + factor to calculate VCM ring periodic time Tvib.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum:
> > + - 0 # Dividing Rate - 2
> > + - 1 # Dividing Rate - 1
> > + - 2 # Dividing Rate - 1/2
> > + - 3 # Dividing Rate - 1/4
> > + - 4 # Dividing Rate - 8
> > + - 5 # Dividing Rate - 4
>
> Same here, you should not need these comments explaining the values,
> use
> an enum with meaningful values please.
>
About "aac-mode/aac-timing/clock-presc", we test this driver with
default settings accroding to SPEC and VCM works well, so I will not
export these property in YMAL and let driver use default settings.
How do you think about it?
> Thanks,
> Conor.
>
> >
> >
^ permalink raw reply
* [PATCH v3 0/6] Add Loongson-2k0500 and Loongson-2k2000 clock support
From: Binbin Zhou @ 2024-04-11 2:58 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu
Cc: Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch, Binbin Zhou
Hi all:
As we know, the Loongson-2K family of SoCs (ls2k0500/ls2k1000/ls2k2000)
have a similar clock structure, and I support them to be configured with
different parameters (e.g., register offsets, etc.).
To make it easier to add support for different SoCs, I refactored the
original driver to make the whole driver as understandable as possible.
Briefly, I have divided all clocks into three categories according to
their properties and their parent clocks: Independent PLLs, clocks based
on frequency scales, and clock dividers.
Thanks.
----
V3:
- Remove the dts-related patches and update dts at once after all relevant
drivers are complete.
patch(1/6):
- Drop unsuitable comments.
Link to V2:
https://lore.kernel.org/all/cover.1711504700.git.zhoubinbin@loongson.cn/
V2:
patch(1/8):
- Drop LOONGSON2_CLK_END, for it is not a binding.
patch(1/8)(3/8)(5/8):
- Add Acked-by tag from Conor, Thanks.
Link to V1:
https://lore.kernel.org/all/cover.1710926402.git.zhoubinbin@loongson.cn/
Binbin Zhou (6):
dt-bindings: clock: Add Loongson-2K expand clock index
clk: clk-loongson2: Refactor driver for adding new platforms
dt-bindings: clock: loongson2: Add Loongson-2K0500 compatible
clk: clk-loongson2: Add Loongson-2K0500 clock support
dt-bindings: clock: loongson2: Add Loongson-2K2000 compatible
clk: clk-loongson2: Add Loongson-2K2000 clock support
.../bindings/clock/loongson,ls2k-clk.yaml | 4 +-
drivers/clk/clk-loongson2.c | 549 ++++++++++--------
include/dt-bindings/clock/loongson,ls2k-clk.h | 54 +-
3 files changed, 329 insertions(+), 278 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v3 1/6] dt-bindings: clock: Add Loongson-2K expand clock index
From: Binbin Zhou @ 2024-04-11 2:58 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu
Cc: Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch, Binbin Zhou, Conor Dooley
In-Reply-To: <cover.1712731524.git.zhoubinbin@loongson.cn>
In the new Loongson-2K family of SoCs, more clock indexes are needed,
such as clock gates.
The patch adds these clock indexes
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
include/dt-bindings/clock/loongson,ls2k-clk.h | 54 ++++++++++++-------
1 file changed, 35 insertions(+), 19 deletions(-)
diff --git a/include/dt-bindings/clock/loongson,ls2k-clk.h b/include/dt-bindings/clock/loongson,ls2k-clk.h
index 3bc4dfc193c2..4279ba595f1e 100644
--- a/include/dt-bindings/clock/loongson,ls2k-clk.h
+++ b/include/dt-bindings/clock/loongson,ls2k-clk.h
@@ -7,24 +7,40 @@
#ifndef __DT_BINDINGS_CLOCK_LOONGSON2_H
#define __DT_BINDINGS_CLOCK_LOONGSON2_H
-#define LOONGSON2_REF_100M 0
-#define LOONGSON2_NODE_PLL 1
-#define LOONGSON2_DDR_PLL 2
-#define LOONGSON2_DC_PLL 3
-#define LOONGSON2_PIX0_PLL 4
-#define LOONGSON2_PIX1_PLL 5
-#define LOONGSON2_NODE_CLK 6
-#define LOONGSON2_HDA_CLK 7
-#define LOONGSON2_GPU_CLK 8
-#define LOONGSON2_DDR_CLK 9
-#define LOONGSON2_GMAC_CLK 10
-#define LOONGSON2_DC_CLK 11
-#define LOONGSON2_APB_CLK 12
-#define LOONGSON2_USB_CLK 13
-#define LOONGSON2_SATA_CLK 14
-#define LOONGSON2_PIX0_CLK 15
-#define LOONGSON2_PIX1_CLK 16
-#define LOONGSON2_BOOT_CLK 17
-#define LOONGSON2_CLK_END 18
+#define LOONGSON2_REF_100M 0
+#define LOONGSON2_NODE_PLL 1
+#define LOONGSON2_DDR_PLL 2
+#define LOONGSON2_DC_PLL 3
+#define LOONGSON2_PIX0_PLL 4
+#define LOONGSON2_PIX1_PLL 5
+#define LOONGSON2_NODE_CLK 6
+#define LOONGSON2_HDA_CLK 7
+#define LOONGSON2_GPU_CLK 8
+#define LOONGSON2_DDR_CLK 9
+#define LOONGSON2_GMAC_CLK 10
+#define LOONGSON2_DC_CLK 11
+#define LOONGSON2_APB_CLK 12
+#define LOONGSON2_USB_CLK 13
+#define LOONGSON2_SATA_CLK 14
+#define LOONGSON2_PIX0_CLK 15
+#define LOONGSON2_PIX1_CLK 16
+#define LOONGSON2_BOOT_CLK 17
+#define LOONGSON2_OUT0_GATE 18
+#define LOONGSON2_GMAC_GATE 19
+#define LOONGSON2_RIO_GATE 20
+#define LOONGSON2_DC_GATE 21
+#define LOONGSON2_GPU_GATE 22
+#define LOONGSON2_DDR_GATE 23
+#define LOONGSON2_HDA_GATE 24
+#define LOONGSON2_NODE_GATE 25
+#define LOONGSON2_EMMC_GATE 26
+#define LOONGSON2_PIX0_GATE 27
+#define LOONGSON2_PIX1_GATE 28
+#define LOONGSON2_OUT0_CLK 29
+#define LOONGSON2_RIO_CLK 30
+#define LOONGSON2_EMMC_CLK 31
+#define LOONGSON2_DES_CLK 32
+#define LOONGSON2_I2S_CLK 33
+#define LOONGSON2_MISC_CLK 34
#endif
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/6] clk: clk-loongson2: Refactor driver for adding new platforms
From: Binbin Zhou @ 2024-04-11 2:58 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu
Cc: Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch, Binbin Zhou
In-Reply-To: <cover.1712731524.git.zhoubinbin@loongson.cn>
The driver only supported loongson-2K1000 at first, but the clock
structure of loongson-2K0500 and loongson-2K2000 are actually similar,
and I tried to refactor the whole driver to adjust to the addition of
the new platform.
Briefly, I have divided all clocks into three categories according to
their properties and their parent clocks: Independent PLLs, clocks based
on frequency scales, and clock dividers.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
drivers/clk/clk-loongson2.c | 459 ++++++++++++++++--------------------
1 file changed, 199 insertions(+), 260 deletions(-)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c
index bacdcbb287ac..ff2ade6a471a 100644
--- a/drivers/clk/clk-loongson2.c
+++ b/drivers/clk/clk-loongson2.c
@@ -6,6 +6,7 @@
#include <linux/err.h>
#include <linux/init.h>
+#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/slab.h>
#include <linux/module.h>
@@ -13,317 +14,254 @@
#include <linux/io-64-nonatomic-lo-hi.h>
#include <dt-bindings/clock/loongson,ls2k-clk.h>
-#define LOONGSON2_PLL_MULT_SHIFT 32
-#define LOONGSON2_PLL_MULT_WIDTH 10
-#define LOONGSON2_PLL_DIV_SHIFT 26
-#define LOONGSON2_PLL_DIV_WIDTH 6
-#define LOONGSON2_APB_FREQSCALE_SHIFT 20
-#define LOONGSON2_APB_FREQSCALE_WIDTH 3
-#define LOONGSON2_USB_FREQSCALE_SHIFT 16
-#define LOONGSON2_USB_FREQSCALE_WIDTH 3
-#define LOONGSON2_SATA_FREQSCALE_SHIFT 12
-#define LOONGSON2_SATA_FREQSCALE_WIDTH 3
-#define LOONGSON2_BOOT_FREQSCALE_SHIFT 8
-#define LOONGSON2_BOOT_FREQSCALE_WIDTH 3
-
-static void __iomem *loongson2_pll_base;
-
static const struct clk_parent_data pdata[] = {
- { .fw_name = "ref_100m",},
+ { .fw_name = "ref_100m", },
};
-static struct clk_hw *loongson2_clk_register(struct device *dev,
- const char *name,
- const char *parent_name,
- const struct clk_ops *ops,
- unsigned long flags)
-{
- int ret;
- struct clk_hw *hw;
- struct clk_init_data init = { };
-
- hw = devm_kzalloc(dev, sizeof(*hw), GFP_KERNEL);
- if (!hw)
- return ERR_PTR(-ENOMEM);
-
- init.name = name;
- init.ops = ops;
- init.flags = flags;
- init.num_parents = 1;
-
- if (!parent_name)
- init.parent_data = pdata;
- else
- init.parent_names = &parent_name;
-
- hw->init = &init;
-
- ret = devm_clk_hw_register(dev, hw);
- if (ret)
- hw = ERR_PTR(ret);
-
- return hw;
-}
-
-static unsigned long loongson2_calc_pll_rate(int offset, unsigned long rate)
-{
- u64 val;
- u32 mult, div;
-
- val = readq(loongson2_pll_base + offset);
-
- mult = (val >> LOONGSON2_PLL_MULT_SHIFT) &
- clk_div_mask(LOONGSON2_PLL_MULT_WIDTH);
- div = (val >> LOONGSON2_PLL_DIV_SHIFT) &
- clk_div_mask(LOONGSON2_PLL_DIV_WIDTH);
-
- return div_u64((u64)rate * mult, div);
-}
-
-static unsigned long loongson2_node_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- return loongson2_calc_pll_rate(0x0, parent_rate);
-}
-
-static const struct clk_ops loongson2_node_clk_ops = {
- .recalc_rate = loongson2_node_recalc_rate,
+enum loongson2_clk_type {
+ CLK_TYPE_PLL,
+ CLK_TYPE_SCALE,
+ CLK_TYPE_DIVIDER,
+ CLK_TYPE_NONE,
};
-static unsigned long loongson2_ddr_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- return loongson2_calc_pll_rate(0x10, parent_rate);
-}
+struct loongson2_clk_provider {
+ void __iomem *base;
+ struct device *dev;
+ struct clk_hw_onecell_data clk_data;
+ spinlock_t clk_lock; /* protect access to DIV registers */
+};
-static const struct clk_ops loongson2_ddr_clk_ops = {
- .recalc_rate = loongson2_ddr_recalc_rate,
+struct loongson2_clk_data {
+ struct clk_hw hw;
+ void __iomem *reg;
+ u8 div_shift;
+ u8 div_width;
+ u8 mult_shift;
+ u8 mult_width;
};
-static unsigned long loongson2_dc_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- return loongson2_calc_pll_rate(0x20, parent_rate);
-}
+struct loongson2_clk_board_info {
+ u8 id;
+ enum loongson2_clk_type type;
+ const char *name;
+ const char *parent_name;
+ u8 reg_offset;
+ u8 div_shift;
+ u8 div_width;
+ u8 mult_shift;
+ u8 mult_width;
+};
-static const struct clk_ops loongson2_dc_clk_ops = {
- .recalc_rate = loongson2_dc_recalc_rate,
+#define CLK_DIV(_id, _name, _pname, _offset, _dshift, _dwidth) \
+ { \
+ .id = _id, \
+ .type = CLK_TYPE_DIVIDER, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .reg_offset = _offset, \
+ .div_shift = _dshift, \
+ .div_width = _dwidth, \
+ }
+
+#define CLK_PLL(_id, _name, _offset, _mshift, _mwidth, \
+ _dshift, _dwidth) \
+ { \
+ .id = _id, \
+ .type = CLK_TYPE_PLL, \
+ .name = _name, \
+ .parent_name = NULL, \
+ .reg_offset = _offset, \
+ .mult_shift = _mshift, \
+ .mult_width = _mwidth, \
+ .div_shift = _dshift, \
+ .div_width = _dwidth, \
+ }
+
+#define CLK_SCALE(_id, _name, _pname, _offset, \
+ _dshift, _dwidth) \
+ { \
+ .id = _id, \
+ .type = CLK_TYPE_SCALE, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .reg_offset = _offset, \
+ .div_shift = _dshift, \
+ .div_width = _dwidth, \
+ }
+
+static const struct loongson2_clk_board_info ls2k1000_clks[] = {
+ CLK_PLL(LOONGSON2_NODE_PLL, "pll_node", 0, 32, 10, 26, 6),
+ CLK_PLL(LOONGSON2_DDR_PLL, "pll_ddr", 0x10, 32, 10, 26, 6),
+ CLK_PLL(LOONGSON2_DC_PLL, "pll_dc", 0x20, 32, 10, 26, 6),
+ CLK_PLL(LOONGSON2_PIX0_PLL, "pll_pix0", 0x30, 32, 10, 26, 6),
+ CLK_PLL(LOONGSON2_PIX1_PLL, "pll_pix1", 0x40, 32, 10, 26, 6),
+ CLK_DIV(LOONGSON2_NODE_CLK, "clk_node", "pll_node", 0x8, 0, 6),
+ CLK_DIV(LOONGSON2_DDR_CLK, "clk_ddr", "pll_ddr", 0x18, 0, 6),
+ CLK_DIV(LOONGSON2_GPU_CLK, "clk_gpu", "pll_ddr", 0x18, 22, 6),
+ /*
+ * The hda clk divisor in the upper 32bits and the clk-prodiver
+ * layer code doesn't support 64bit io operation thus a conversion
+ * is required that subtract shift by 32 and add 4byte to the hda
+ * address
+ */
+ CLK_DIV(LOONGSON2_HDA_CLK, "clk_hda", "pll_ddr", 0x22, 12, 7),
+ CLK_DIV(LOONGSON2_DC_CLK, "clk_dc", "pll_dc", 0x28, 0, 6),
+ CLK_DIV(LOONGSON2_GMAC_CLK, "clk_gmac", "pll_dc", 0x28, 22, 6),
+ CLK_DIV(LOONGSON2_PIX0_CLK, "clk_pix0", "pll_pix0", 0x38, 0, 6),
+ CLK_DIV(LOONGSON2_PIX1_CLK, "clk_pix1", "pll_pix1", 0x38, 0, 6),
+ CLK_SCALE(LOONGSON2_BOOT_CLK, "clk_boot", NULL, 0x50, 8, 3),
+ CLK_SCALE(LOONGSON2_SATA_CLK, "clk_sata", "clk_gmac", 0x50, 12, 3),
+ CLK_SCALE(LOONGSON2_USB_CLK, "clk_usb", "clk_gmac", 0x50, 16, 3),
+ CLK_SCALE(LOONGSON2_APB_CLK, "clk_apb", "clk_gmac", 0x50, 20, 3),
+ { /* Sentinel */ },
};
-static unsigned long loongson2_pix0_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
+static inline struct loongson2_clk_data *to_loongson2_clk(struct clk_hw *hw)
{
- return loongson2_calc_pll_rate(0x30, parent_rate);
+ return container_of(hw, struct loongson2_clk_data, hw);
}
-static const struct clk_ops loongson2_pix0_clk_ops = {
- .recalc_rate = loongson2_pix0_recalc_rate,
-};
-
-static unsigned long loongson2_pix1_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
+static inline unsigned long loongson2_rate_part(u64 val, u8 shift, u8 width)
{
- return loongson2_calc_pll_rate(0x40, parent_rate);
+ return (val & GENMASK(shift + width - 1, shift)) >> shift;
}
-static const struct clk_ops loongson2_pix1_clk_ops = {
- .recalc_rate = loongson2_pix1_recalc_rate,
-};
-
-static unsigned long loongson2_calc_rate(unsigned long rate,
- int shift, int width)
+static unsigned long loongson2_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
{
- u64 val;
- u32 mult;
-
- val = readq(loongson2_pll_base + 0x50);
+ u64 val, mult, div;
+ struct loongson2_clk_data *clk = to_loongson2_clk(hw);
- mult = (val >> shift) & clk_div_mask(width);
+ val = readq(clk->reg);
+ mult = loongson2_rate_part(val, clk->mult_shift, clk->mult_width);
+ div = loongson2_rate_part(val, clk->div_shift, clk->div_width);
- return div_u64((u64)rate * (mult + 1), 8);
-}
-
-static unsigned long loongson2_boot_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- return loongson2_calc_rate(parent_rate,
- LOONGSON2_BOOT_FREQSCALE_SHIFT,
- LOONGSON2_BOOT_FREQSCALE_WIDTH);
+ return div_u64((u64)parent_rate * mult, div);
}
-static const struct clk_ops loongson2_boot_clk_ops = {
- .recalc_rate = loongson2_boot_recalc_rate,
+static const struct clk_ops loongson2_pll_recalc_ops = {
+ .recalc_rate = loongson2_pll_recalc_rate,
};
-static unsigned long loongson2_apb_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
+static unsigned long loongson2_freqscale_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
{
- return loongson2_calc_rate(parent_rate,
- LOONGSON2_APB_FREQSCALE_SHIFT,
- LOONGSON2_APB_FREQSCALE_WIDTH);
-}
+ u64 val, mult;
+ struct loongson2_clk_data *clk = to_loongson2_clk(hw);
-static const struct clk_ops loongson2_apb_clk_ops = {
- .recalc_rate = loongson2_apb_recalc_rate,
-};
+ val = readq(clk->reg);
+ mult = loongson2_rate_part(val, clk->div_shift, clk->div_width) + 1;
-static unsigned long loongson2_usb_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- return loongson2_calc_rate(parent_rate,
- LOONGSON2_USB_FREQSCALE_SHIFT,
- LOONGSON2_USB_FREQSCALE_WIDTH);
+ return div_u64((u64)parent_rate * mult, 8);
}
-static const struct clk_ops loongson2_usb_clk_ops = {
- .recalc_rate = loongson2_usb_recalc_rate,
+static const struct clk_ops loongson2_freqscale_recalc_ops = {
+ .recalc_rate = loongson2_freqscale_recalc_rate,
};
-static unsigned long loongson2_sata_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
+static struct clk_hw *loongson2_clk_register(struct loongson2_clk_provider *clp,
+ const struct loongson2_clk_board_info *cld,
+ const struct clk_ops *ops)
{
- return loongson2_calc_rate(parent_rate,
- LOONGSON2_SATA_FREQSCALE_SHIFT,
- LOONGSON2_SATA_FREQSCALE_WIDTH);
-}
+ int ret;
+ struct clk_hw *hw;
+ struct loongson2_clk_data *clk;
+ struct clk_init_data init = { };
-static const struct clk_ops loongson2_sata_clk_ops = {
- .recalc_rate = loongson2_sata_recalc_rate,
-};
+ clk = devm_kzalloc(clp->dev, sizeof(*clk), GFP_KERNEL);
+ if (!clk)
+ return ERR_PTR(-ENOMEM);
-static inline int loongson2_check_clk_hws(struct clk_hw *clks[], unsigned int count)
-{
- unsigned int i;
+ init.name = cld->name;
+ init.ops = ops;
+ init.flags = 0;
+ init.num_parents = 1;
- for (i = 0; i < count; i++)
- if (IS_ERR(clks[i])) {
- pr_err("Loongson2 clk %u: register failed with %ld\n",
- i, PTR_ERR(clks[i]));
- return PTR_ERR(clks[i]);
- }
+ if (!cld->parent_name)
+ init.parent_data = pdata;
+ else
+ init.parent_names = &cld->parent_name;
+
+ clk->reg = clp->base + cld->reg_offset;
+ clk->div_shift = cld->div_shift;
+ clk->div_width = cld->div_width;
+ clk->mult_shift = cld->mult_shift;
+ clk->mult_width = cld->mult_width;
+ clk->hw.init = &init;
- return 0;
+ hw = &clk->hw;
+ ret = devm_clk_hw_register(clp->dev, hw);
+ if (ret)
+ clk = ERR_PTR(ret);
+
+ return hw;
}
static int loongson2_clk_probe(struct platform_device *pdev)
{
- int ret;
- struct clk_hw **hws;
- struct clk_hw_onecell_data *clk_hw_data;
- spinlock_t loongson2_clk_lock;
+ int i, clks_num = 0;
+ struct clk_hw *hw;
struct device *dev = &pdev->dev;
+ struct loongson2_clk_provider *clp;
+ const struct loongson2_clk_board_info *p, *data;
- loongson2_pll_base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(loongson2_pll_base))
- return PTR_ERR(loongson2_pll_base);
-
- clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws, LOONGSON2_CLK_END),
- GFP_KERNEL);
- if (WARN_ON(!clk_hw_data))
- return -ENOMEM;
-
- clk_hw_data->num = LOONGSON2_CLK_END;
- hws = clk_hw_data->hws;
-
- hws[LOONGSON2_NODE_PLL] = loongson2_clk_register(dev, "node_pll",
- NULL,
- &loongson2_node_clk_ops, 0);
-
- hws[LOONGSON2_DDR_PLL] = loongson2_clk_register(dev, "ddr_pll",
- NULL,
- &loongson2_ddr_clk_ops, 0);
+ data = device_get_match_data(dev);
+ if (!data)
+ return -EINVAL;
- hws[LOONGSON2_DC_PLL] = loongson2_clk_register(dev, "dc_pll",
- NULL,
- &loongson2_dc_clk_ops, 0);
+ for (p = data; p->name; p++)
+ clks_num++;
- hws[LOONGSON2_PIX0_PLL] = loongson2_clk_register(dev, "pix0_pll",
- NULL,
- &loongson2_pix0_clk_ops, 0);
-
- hws[LOONGSON2_PIX1_PLL] = loongson2_clk_register(dev, "pix1_pll",
- NULL,
- &loongson2_pix1_clk_ops, 0);
+ clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num),
+ GFP_KERNEL);
+ if (!clp)
+ return -ENOMEM;
- hws[LOONGSON2_BOOT_CLK] = loongson2_clk_register(dev, "boot",
- NULL,
- &loongson2_boot_clk_ops, 0);
+ clp->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(clp->base))
+ return PTR_ERR(clp->base);
+
+ spin_lock_init(&clp->clk_lock);
+ clp->clk_data.num = clks_num + 1;
+ clp->dev = dev;
+
+ for (i = 0; i < clks_num; i++) {
+ p = &data[i];
+ switch (p->type) {
+ case CLK_TYPE_PLL:
+ hw = loongson2_clk_register(clp, p,
+ &loongson2_pll_recalc_ops);
+ break;
+ case CLK_TYPE_SCALE:
+ hw = loongson2_clk_register(clp, p,
+ &loongson2_freqscale_recalc_ops);
+ break;
+ case CLK_TYPE_DIVIDER:
+ hw = devm_clk_hw_register_divider(dev, p->name,
+ p->parent_name, 0,
+ clp->base + p->reg_offset,
+ p->div_shift, p->div_width,
+ CLK_DIVIDER_ONE_BASED,
+ &clp->clk_lock);
+ break;
+ default:
+ return dev_err_probe(dev, -EINVAL, "Invalid clk type\n");
+ }
- hws[LOONGSON2_NODE_CLK] = devm_clk_hw_register_divider(dev, "node",
- "node_pll", 0,
- loongson2_pll_base + 0x8, 0,
- 6, CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
+ if (IS_ERR(hw))
+ return dev_err_probe(dev, PTR_ERR(hw),
+ "Register clk: %s, type: %u failed!\n",
+ p->name, p->type);
- /*
- * The hda clk divisor in the upper 32bits and the clk-prodiver
- * layer code doesn't support 64bit io operation thus a conversion
- * is required that subtract shift by 32 and add 4byte to the hda
- * address
- */
- hws[LOONGSON2_HDA_CLK] = devm_clk_hw_register_divider(dev, "hda",
- "ddr_pll", 0,
- loongson2_pll_base + 0x22, 12,
- 7, CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
-
- hws[LOONGSON2_GPU_CLK] = devm_clk_hw_register_divider(dev, "gpu",
- "ddr_pll", 0,
- loongson2_pll_base + 0x18, 22,
- 6, CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
-
- hws[LOONGSON2_DDR_CLK] = devm_clk_hw_register_divider(dev, "ddr",
- "ddr_pll", 0,
- loongson2_pll_base + 0x18, 0,
- 6, CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
-
- hws[LOONGSON2_GMAC_CLK] = devm_clk_hw_register_divider(dev, "gmac",
- "dc_pll", 0,
- loongson2_pll_base + 0x28, 22,
- 6, CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
-
- hws[LOONGSON2_DC_CLK] = devm_clk_hw_register_divider(dev, "dc",
- "dc_pll", 0,
- loongson2_pll_base + 0x28, 0,
- 6, CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
-
- hws[LOONGSON2_APB_CLK] = loongson2_clk_register(dev, "apb",
- "gmac",
- &loongson2_apb_clk_ops, 0);
-
- hws[LOONGSON2_USB_CLK] = loongson2_clk_register(dev, "usb",
- "gmac",
- &loongson2_usb_clk_ops, 0);
-
- hws[LOONGSON2_SATA_CLK] = loongson2_clk_register(dev, "sata",
- "gmac",
- &loongson2_sata_clk_ops, 0);
-
- hws[LOONGSON2_PIX0_CLK] = clk_hw_register_divider(NULL, "pix0",
- "pix0_pll", 0,
- loongson2_pll_base + 0x38, 0, 6,
- CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
-
- hws[LOONGSON2_PIX1_CLK] = clk_hw_register_divider(NULL, "pix1",
- "pix1_pll", 0,
- loongson2_pll_base + 0x48, 0, 6,
- CLK_DIVIDER_ONE_BASED,
- &loongson2_clk_lock);
-
- ret = loongson2_check_clk_hws(hws, LOONGSON2_CLK_END);
- if (ret)
- return ret;
+ clp->clk_data.hws[p->id] = hw;
+ }
- return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_hw_data);
+ return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, &clp->clk_data);
}
static const struct of_device_id loongson2_clk_match_table[] = {
- { .compatible = "loongson,ls2k-clk" },
+ { .compatible = "loongson,ls2k-clk", .data = &ls2k1000_clks },
{ }
};
MODULE_DEVICE_TABLE(of, loongson2_clk_match_table);
@@ -338,4 +276,5 @@ static struct platform_driver loongson2_clk_driver = {
module_platform_driver(loongson2_clk_driver);
MODULE_DESCRIPTION("Loongson2 clock driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited");
MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/6] dt-bindings: clock: loongson2: Add Loongson-2K0500 compatible
From: Binbin Zhou @ 2024-04-11 2:58 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu
Cc: Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch, Binbin Zhou, Conor Dooley
In-Reply-To: <cover.1712731524.git.zhoubinbin@loongson.cn>
Add the devicetree compatible for Loongson-2K0500 clocks.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml b/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
index 63a59015987e..83baee40e200 100644
--- a/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
+++ b/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
@@ -16,7 +16,8 @@ description: |
properties:
compatible:
enum:
- - loongson,ls2k-clk
+ - loongson,ls2k0500-clk
+ - loongson,ls2k-clk # This is for Loongson-2K1000
reg:
maxItems: 1
--
2.43.0
^ permalink raw reply related
* [PATCH v3 4/6] clk: clk-loongson2: Add Loongson-2K0500 clock support
From: Binbin Zhou @ 2024-04-11 2:58 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu
Cc: Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch, Binbin Zhou
In-Reply-To: <cover.1712731524.git.zhoubinbin@loongson.cn>
The Loongson-2K0500 and Loongson-2K1000 clock is similar, we add its
support by different configurations.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
drivers/clk/clk-loongson2.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c
index ff2ade6a471a..b5dd9d028826 100644
--- a/drivers/clk/clk-loongson2.c
+++ b/drivers/clk/clk-loongson2.c
@@ -90,6 +90,27 @@ struct loongson2_clk_board_info {
.div_width = _dwidth, \
}
+static const struct loongson2_clk_board_info ls2k0500_clks[] = {
+ CLK_PLL(LOONGSON2_NODE_PLL, "pll_node", 0, 16, 8, 8, 6),
+ CLK_PLL(LOONGSON2_DDR_PLL, "pll_ddr", 0x8, 16, 8, 8, 6),
+ CLK_PLL(LOONGSON2_DC_PLL, "pll_soc", 0x10, 16, 8, 8, 6),
+ CLK_PLL(LOONGSON2_PIX0_PLL, "pll_pix0", 0x18, 16, 8, 8, 6),
+ CLK_PLL(LOONGSON2_PIX1_PLL, "pll_pix1", 0x20, 16, 8, 8, 6),
+ CLK_DIV(LOONGSON2_NODE_CLK, "clk_node", "pll_node", 0, 24, 6),
+ CLK_DIV(LOONGSON2_DDR_CLK, "clk_ddr", "pll_ddr", 0x8, 24, 6),
+ CLK_DIV(LOONGSON2_HDA_CLK, "clk_hda", "pll_ddr", 0xc, 8, 6),
+ CLK_DIV(LOONGSON2_GPU_CLK, "clk_gpu", "pll_soc", 0x10, 24, 6),
+ CLK_DIV(LOONGSON2_DC_CLK, "clk_sb", "pll_soc", 0x14, 0, 6),
+ CLK_DIV(LOONGSON2_GMAC_CLK, "clk_gmac", "pll_soc", 0x14, 8, 6),
+ CLK_DIV(LOONGSON2_PIX0_CLK, "clk_pix0", "pll_pix0", 0x18, 24, 6),
+ CLK_DIV(LOONGSON2_PIX1_CLK, "clk_pix1", "pll_pix1", 0x20, 24, 6),
+ CLK_SCALE(LOONGSON2_BOOT_CLK, "clk_boot", "clk_sb", 0x28, 8, 3),
+ CLK_SCALE(LOONGSON2_SATA_CLK, "clk_sata", "clk_sb", 0x28, 12, 3),
+ CLK_SCALE(LOONGSON2_USB_CLK, "clk_usb", "clk_sb", 0x28, 16, 3),
+ CLK_SCALE(LOONGSON2_APB_CLK, "clk_apb", "clk_sb", 0x28, 20, 3),
+ { /* Sentinel */ },
+};
+
static const struct loongson2_clk_board_info ls2k1000_clks[] = {
CLK_PLL(LOONGSON2_NODE_PLL, "pll_node", 0, 32, 10, 26, 6),
CLK_PLL(LOONGSON2_DDR_PLL, "pll_ddr", 0x10, 32, 10, 26, 6),
@@ -261,6 +282,7 @@ static int loongson2_clk_probe(struct platform_device *pdev)
}
static const struct of_device_id loongson2_clk_match_table[] = {
+ { .compatible = "loongson,ls2k0500-clk", .data = &ls2k0500_clks },
{ .compatible = "loongson,ls2k-clk", .data = &ls2k1000_clks },
{ }
};
--
2.43.0
^ permalink raw reply related
* [PATCH v3 5/6] dt-bindings: clock: loongson2: Add Loongson-2K2000 compatible
From: Binbin Zhou @ 2024-04-11 2:58 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu
Cc: Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch, Binbin Zhou, Conor Dooley
In-Reply-To: <cover.1712731524.git.zhoubinbin@loongson.cn>
Add the devicetree compatible for Loongson-2K2000 clocks.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml b/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
index 83baee40e200..4f79cdb417ab 100644
--- a/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
+++ b/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
@@ -18,6 +18,7 @@ properties:
enum:
- loongson,ls2k0500-clk
- loongson,ls2k-clk # This is for Loongson-2K1000
+ - loongson,ls2k2000-clk
reg:
maxItems: 1
--
2.43.0
^ permalink raw reply related
* [PATCH v3 6/6] clk: clk-loongson2: Add Loongson-2K2000 clock support
From: Binbin Zhou @ 2024-04-11 2:58 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu
Cc: Huacai Chen, loongson-kernel, linux-clk, devicetree, Xuerui Wang,
loongarch, Binbin Zhou
In-Reply-To: <cover.1712731524.git.zhoubinbin@loongson.cn>
The Loongson-2K2000 and Loongson-2K1000 clock is similar, we add its
support by different configurations.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
drivers/clk/clk-loongson2.c | 72 +++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c
index b5dd9d028826..0d82aeeadb45 100644
--- a/drivers/clk/clk-loongson2.c
+++ b/drivers/clk/clk-loongson2.c
@@ -22,6 +22,8 @@ enum loongson2_clk_type {
CLK_TYPE_PLL,
CLK_TYPE_SCALE,
CLK_TYPE_DIVIDER,
+ CLK_TYPE_GATE,
+ CLK_TYPE_FIXED,
CLK_TYPE_NONE,
};
@@ -46,11 +48,13 @@ struct loongson2_clk_board_info {
enum loongson2_clk_type type;
const char *name;
const char *parent_name;
+ unsigned long fixed_rate;
u8 reg_offset;
u8 div_shift;
u8 div_width;
u8 mult_shift;
u8 mult_width;
+ u8 bit_idx;
};
#define CLK_DIV(_id, _name, _pname, _offset, _dshift, _dwidth) \
@@ -90,6 +94,25 @@ struct loongson2_clk_board_info {
.div_width = _dwidth, \
}
+#define CLK_GATE(_id, _name, _pname, _offset, _bidx) \
+ { \
+ .id = _id, \
+ .type = CLK_TYPE_GATE, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .reg_offset = _offset, \
+ .bit_idx = _bidx, \
+ }
+
+#define CLK_FIXED(_id, _name, _pname, _rate) \
+ { \
+ .id = _id, \
+ .type = CLK_TYPE_FIXED, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .fixed_rate = _rate, \
+ }
+
static const struct loongson2_clk_board_info ls2k0500_clks[] = {
CLK_PLL(LOONGSON2_NODE_PLL, "pll_node", 0, 16, 8, 8, 6),
CLK_PLL(LOONGSON2_DDR_PLL, "pll_ddr", 0x8, 16, 8, 8, 6),
@@ -138,6 +161,44 @@ static const struct loongson2_clk_board_info ls2k1000_clks[] = {
{ /* Sentinel */ },
};
+static const struct loongson2_clk_board_info ls2k2000_clks[] = {
+ CLK_PLL(LOONGSON2_DC_PLL, "pll_0", 0, 21, 9, 32, 6),
+ CLK_PLL(LOONGSON2_DDR_PLL, "pll_1", 0x10, 21, 9, 32, 6),
+ CLK_PLL(LOONGSON2_NODE_PLL, "pll_2", 0x20, 21, 9, 32, 6),
+ CLK_PLL(LOONGSON2_PIX0_PLL, "pll_pix0", 0x30, 21, 9, 32, 6),
+ CLK_PLL(LOONGSON2_PIX1_PLL, "pll_pix1", 0x40, 21, 9, 32, 6),
+ CLK_GATE(LOONGSON2_OUT0_GATE, "out0_gate", "pll_0", 0, 40),
+ CLK_GATE(LOONGSON2_GMAC_GATE, "gmac_gate", "pll_0", 0, 41),
+ CLK_GATE(LOONGSON2_RIO_GATE, "rio_gate", "pll_0", 0, 42),
+ CLK_GATE(LOONGSON2_DC_GATE, "dc_gate", "pll_1", 0x10, 40),
+ CLK_GATE(LOONGSON2_DDR_GATE, "ddr_gate", "pll_1", 0x10, 41),
+ CLK_GATE(LOONGSON2_GPU_GATE, "gpu_gate", "pll_1", 0x10, 42),
+ CLK_GATE(LOONGSON2_HDA_GATE, "hda_gate", "pll_2", 0x20, 40),
+ CLK_GATE(LOONGSON2_NODE_GATE, "node_gate", "pll_2", 0x20, 41),
+ CLK_GATE(LOONGSON2_EMMC_GATE, "emmc_gate", "pll_2", 0x20, 42),
+ CLK_GATE(LOONGSON2_PIX0_GATE, "pix0_gate", "pll_pix0", 0x30, 40),
+ CLK_GATE(LOONGSON2_PIX1_GATE, "pix1_gate", "pll_pix1", 0x40, 40),
+ CLK_DIV(LOONGSON2_OUT0_CLK, "clk_out0", "out0_gate", 0, 0, 6),
+ CLK_DIV(LOONGSON2_GMAC_CLK, "clk_gmac", "gmac_gate", 0, 7, 6),
+ CLK_DIV(LOONGSON2_RIO_CLK, "clk_rio", "rio_gate", 0, 14, 6),
+ CLK_DIV(LOONGSON2_DC_CLK, "clk_dc", "dc_gate", 0x10, 0, 6),
+ CLK_DIV(LOONGSON2_GPU_CLK, "clk_gpu", "gpu_gate", 0x10, 7, 6),
+ CLK_DIV(LOONGSON2_DDR_CLK, "clk_ddr", "ddr_gate", 0x10, 14, 6),
+ CLK_DIV(LOONGSON2_HDA_CLK, "clk_hda", "hda_gate", 0x20, 0, 6),
+ CLK_DIV(LOONGSON2_NODE_CLK, "clk_node", "node_gate", 0x20, 7, 6),
+ CLK_DIV(LOONGSON2_EMMC_CLK, "clk_emmc", "emmc_gate", 0x20, 14, 6),
+ CLK_DIV(LOONGSON2_PIX0_CLK, "clk_pix0", "pll_pix0", 0x30, 0, 6),
+ CLK_DIV(LOONGSON2_PIX1_CLK, "clk_pix1", "pll_pix1", 0x40, 0, 6),
+ CLK_SCALE(LOONGSON2_SATA_CLK, "clk_sata", "clk_out0", 0x50, 12, 3),
+ CLK_SCALE(LOONGSON2_USB_CLK, "clk_usb", "clk_out0", 0x50, 16, 3),
+ CLK_SCALE(LOONGSON2_APB_CLK, "clk_apb", "clk_node", 0x50, 20, 3),
+ CLK_SCALE(LOONGSON2_BOOT_CLK, "clk_boot", NULL, 0x50, 23, 3),
+ CLK_SCALE(LOONGSON2_DES_CLK, "clk_des", "clk_node", 0x50, 40, 3),
+ CLK_SCALE(LOONGSON2_I2S_CLK, "clk_i2s", "clk_node", 0x50, 44, 3),
+ CLK_FIXED(LOONGSON2_MISC_CLK, "clk_misc", NULL, 50000000),
+ { /* Sentinel */ },
+};
+
static inline struct loongson2_clk_data *to_loongson2_clk(struct clk_hw *hw)
{
return container_of(hw, struct loongson2_clk_data, hw);
@@ -266,6 +327,16 @@ static int loongson2_clk_probe(struct platform_device *pdev)
CLK_DIVIDER_ONE_BASED,
&clp->clk_lock);
break;
+ case CLK_TYPE_GATE:
+ hw = devm_clk_hw_register_gate(dev, p->name, p->parent_name, 0,
+ clp->base + p->reg_offset,
+ p->bit_idx, 0,
+ &clp->clk_lock);
+ break;
+ case CLK_TYPE_FIXED:
+ hw = clk_hw_register_fixed_rate_parent_data(dev, p->name, pdata,
+ 0, p->fixed_rate);
+ break;
default:
return dev_err_probe(dev, -EINVAL, "Invalid clk type\n");
}
@@ -284,6 +355,7 @@ static int loongson2_clk_probe(struct platform_device *pdev)
static const struct of_device_id loongson2_clk_match_table[] = {
{ .compatible = "loongson,ls2k0500-clk", .data = &ls2k0500_clks },
{ .compatible = "loongson,ls2k-clk", .data = &ls2k1000_clks },
+ { .compatible = "loongson,ls2k2000-clk", .data = &ls2k2000_clks },
{ }
};
MODULE_DEVICE_TABLE(of, loongson2_clk_match_table);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 04/11] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag
From: Stephen Boyd @ 2024-04-11 3:06 UTC (permalink / raw)
To: Conor Dooley, Krzysztof Kozlowski, Linus Walleij,
Michael Turquette, Philipp Zabel, Rob Herring, Théo Lebrun
Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
Tawfik Bayouk, Théo Lebrun
In-Reply-To: <20240410-mbly-olb-v1-4-335e496d7be3@bootlin.com>
Quoting Théo Lebrun (2024-04-10 10:12:33)
> index 4a537260f655..cb348e502e41 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -675,13 +675,15 @@ struct clk_div_table {
> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
> * for the divider register. Setting this flag makes the register accesses
> * big endian.
> + * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
> + * Formula is 2 * (value read from hardware + 1).
> */
> struct clk_divider {
> struct clk_hw hw;
> void __iomem *reg;
> u8 shift;
> u8 width;
> - u8 flags;
> + u16 flags;
This can stay u8
> const struct clk_div_table *table;
> spinlock_t *lock;
> };
We should add a kunit test.
^ permalink raw reply
* Re: [PATCH 05/11] clk: eyeq: add driver
From: Stephen Boyd @ 2024-04-11 3:22 UTC (permalink / raw)
To: Conor Dooley, Krzysztof Kozlowski, Linus Walleij,
Michael Turquette, Philipp Zabel, Rob Herring, Théo Lebrun
Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
Tawfik Bayouk, Théo Lebrun
In-Reply-To: <20240410-mbly-olb-v1-5-335e496d7be3@bootlin.com>
Quoting Théo Lebrun (2024-04-10 10:12:34)
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 50af5fc7f570..1eb6e70977a3 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -218,6 +218,17 @@ config COMMON_CLK_EN7523
> This driver provides the fixed clocks and gates present on Airoha
> ARM silicon.
>
> +config COMMON_CLK_EYEQ
> + bool "Clock driver for the Mobileye EyeQ platform"
> + depends on OF || COMPILE_TEST
The OF build dependency looks useless as we have the MACH_ dependency
below.
> + depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
> + default MACH_EYEQ5 || MACH_EYEQ6H
> + help
> + This driver provides clocks found on Mobileye EyeQ5, EyeQ6L and Eye6H
> + SoCs. Controllers live in shared register regions called OLB. Driver
> + provides read-only PLLs, derived from the main crystal clock (which
> + must be constant). It also exposes some divider clocks.
> +
> config COMMON_CLK_FSL_FLEXSPI
> tristate "Clock driver for FlexSPI on Layerscape SoCs"
> depends on ARCH_LAYERSCAPE || COMPILE_TEST
> diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> new file mode 100644
> index 000000000000..bb2535010ae6
> --- /dev/null
> +++ b/drivers/clk/clk-eyeq.c
> @@ -0,0 +1,644 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
> + *
> + * This controller handles read-only PLLs, all derived from the same main
> + * crystal clock. It also exposes divider clocks, those are children to PLLs.
> + * Parent clock is expected to be constant. This driver's registers live in
> + * a shared region called OLB. Some PLLs are initialised early by of_clk_init().
Is OLB a different DT node? It sounds like maybe this is trying to jam a
driver into DT when the OLB node should be a #clock-cells node.
> + *
> + * We use eqc_ as prefix, as-in "EyeQ Clock", but way shorter.
> + *
> + * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
> + */
> +
> +#define pr_fmt(fmt) "clk-eyeq: " fmt
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/overflow.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
> +
> +#define EQC_MAX_DIV_COUNT 4
> +
> +/* In frac mode, it enables fractional noise canceling DAC. Else, no function. */
> +#define PCSR0_DAC_EN BIT(0)
> +/* Fractional or integer mode */
> +#define PCSR0_DSM_EN BIT(1)
> +#define PCSR0_PLL_EN BIT(2)
> +/* All clocks output held at 0 */
> +#define PCSR0_FOUTPOSTDIV_EN BIT(3)
> +#define PCSR0_POST_DIV1 GENMASK(6, 4)
> +#define PCSR0_POST_DIV2 GENMASK(9, 7)
> +#define PCSR0_REF_DIV GENMASK(15, 10)
> +#define PCSR0_INTIN GENMASK(27, 16)
> +#define PCSR0_BYPASS BIT(28)
> +/* Bits 30..29 are reserved */
> +#define PCSR0_PLL_LOCKED BIT(31)
> +
> +#define PCSR1_RESET BIT(0)
> +#define PCSR1_SSGC_DIV GENMASK(4, 1)
> +/* Spread amplitude (% = 0.1 * SPREAD[4:0]) */
> +#define PCSR1_SPREAD GENMASK(9, 5)
> +#define PCSR1_DIS_SSCG BIT(10)
> +/* Down-spread or center-spread */
> +#define PCSR1_DOWN_SPREAD BIT(11)
> +#define PCSR1_FRAC_IN GENMASK(31, 12)
> +
> +/*
> + * Driver might register clock provider from eqc_init() if PLLs are required
> + * early (before platform bus is ready). Store struct eqc_priv inside linked
> + * list to pass clock provider from eqc_init() to eqc_probe() and register
> + * remaining clocks from platform device probe.
> + *
> + * Clock provider is NOT created by eqc_init() if no early clock is required.
> + * Store as linked list because EyeQ6H has multiple clock controller instances.
> + * Matching is done based on devicetree node pointer.
> + */
> +static DEFINE_SPINLOCK(eqc_list_slock);
> +static LIST_HEAD(eqc_list);
> +
> +struct eqc_pll {
> + unsigned int index;
> + const char *name;
> + u32 reg64;
> +};
> +
> +/*
> + * Divider clock. Divider is 2*(v+1), with v the register value.
> + * Min divider is 2, max is 2*(2^width).
> + */
> +struct eqc_div {
> + unsigned int index;
> + const char *name;
> + unsigned int parent;
> + const char *resource_name;
> + u8 shift;
> + u8 width;
> +};
> +
> +struct eqc_match_data {
> + unsigned int early_pll_count;
> + const struct eqc_pll *early_plls;
> +
> + unsigned int pll_count;
> + const struct eqc_pll *plls;
> +
> + unsigned int div_count;
> + const struct eqc_div *divs;
> +};
> +
> +struct eqc_priv {
> + struct clk_hw_onecell_data *cells;
> + const struct eqc_match_data *data;
> + void __iomem *base_plls;
> + struct device_node *np;
> + struct list_head list;
> +};
> +
> +static int eqc_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
> + unsigned long *div, unsigned long *acc)
> +{
> + if (r0 & PCSR0_BYPASS) {
> + *mult = 1;
> + *div = 1;
> + *acc = 0;
> + return 0;
> + }
> +
> + if (!(r0 & PCSR0_PLL_LOCKED))
> + return -EINVAL;
> +
> + *mult = FIELD_GET(PCSR0_INTIN, r0);
> + *div = FIELD_GET(PCSR0_REF_DIV, r0);
> + if (r0 & PCSR0_FOUTPOSTDIV_EN)
> + *div *= FIELD_GET(PCSR0_POST_DIV1, r0) * FIELD_GET(PCSR0_POST_DIV2, r0);
> +
> + /* Fractional mode, in 2^20 (0x100000) parts. */
> + if (r0 & PCSR0_DSM_EN) {
> + *div *= 0x100000;
> + *mult = *mult * 0x100000 + FIELD_GET(PCSR1_FRAC_IN, r1);
> + }
> +
> + if (!*mult || !*div)
> + return -EINVAL;
> +
> + /* Spread spectrum. */
> + if (!(r1 & (PCSR1_RESET | PCSR1_DIS_SSCG))) {
> + /*
> + * Spread is 1/1000 parts of frequency, accuracy is half of
> + * that. To get accuracy, convert to ppb (parts per billion).
> + */
> + u32 spread = FIELD_GET(PCSR1_SPREAD, r1);
> +
> + *acc = spread * 500000;
> + if (r1 & PCSR1_DOWN_SPREAD) {
> + /*
> + * Downspreading: the central frequency is half a
> + * spread lower.
> + */
> + *mult *= 2000 - spread;
> + *div *= 2000;
> + }
> + } else {
> + *acc = 0;
> + }
> +
> + return 0;
> +}
> +
> +static unsigned int eqc_compute_clock_count(const struct eqc_match_data *data)
> +{
> + unsigned int i, nb_clks = 0;
> +
> + for (i = 0; i < data->early_pll_count; i++)
> + if (data->early_plls[i].index >= nb_clks)
> + nb_clks = data->early_plls[i].index + 1;
> + for (i = 0; i < data->pll_count; i++)
> + if (data->plls[i].index >= nb_clks)
> + nb_clks = data->plls[i].index + 1;
> + for (i = 0; i < data->div_count; i++)
> + if (data->divs[i].index >= nb_clks)
> + nb_clks = data->divs[i].index + 1;
> +
> + /* We expect the biggest clock index to be 1 below the clock count. */
> + WARN_ON(nb_clks != data->early_pll_count + data->pll_count + data->div_count);
> +
> + return nb_clks;
> +}
> +
> +static int eqc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + void __iomem *div_resources[EQC_MAX_DIV_COUNT];
> + struct device_node *np = dev->of_node;
> + const struct eqc_match_data *data;
> + struct eqc_priv *priv = NULL;
> + struct clk_hw *hw;
> + unsigned int i;
> +
> + data = device_get_match_data(dev);
> + if (!data)
> + return -ENODEV;
> +
> + if (data->early_pll_count) {
> + /* Device got inited early. Retrieve clock provider from list. */
> + struct eqc_priv *entry;
> +
> + spin_lock(&eqc_list_slock);
> + list_for_each_entry(entry, &eqc_list, list) {
> + if (entry->np == np) {
> + priv = entry;
> + break;
> + }
> + }
> + spin_unlock(&eqc_list_slock);
> +
> + if (!priv)
> + return -ENODEV;
This can be a sub-function.
> + } else {
> + /* Device did NOT get init early. Do it now. */
> + unsigned int nb_clks;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->np = np;
> + priv->data = data;
> +
> + nb_clks = eqc_compute_clock_count(data);
> + priv->cells = devm_kzalloc(dev, struct_size(priv->cells, hws, nb_clks),
> + GFP_KERNEL);
> + if (!priv->cells)
> + return -ENOMEM;
> +
> + priv->cells->num = nb_clks;
> +
> + /*
> + * We expect named resources if divider clocks are present.
> + * Else, we only expect one resource.
> + */
> + if (data->div_count)
> + priv->base_plls = devm_platform_ioremap_resource_byname(pdev, "plls");
> + else
> + priv->base_plls = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(priv->base_plls))
> + return PTR_ERR(priv->base_plls);
> + }
> +
> + for (i = 0; i < data->pll_count; i++) {
> + const struct eqc_pll *pll = &data->plls[i];
> + unsigned long mult, div, acc;
> + u32 r0, r1;
> + u64 val;
> + int ret;
All variables should be declared at the start of the function. Once it
becomes "too heavy" you can split it up into smaller functions, that
again have all variables declared at the start of the function.
> +
> + val = readq(priv->base_plls + pll->reg64);
> + r0 = val;
> + r1 = val >> 32;
> +
> + ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
> + if (ret) {
> + dev_warn(dev, "failed parsing state of %s\n", pll->name);
> + priv->cells->hws[pll->index] = ERR_PTR(ret);
> + continue;
> + }
> +
> + hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev,
> + dev->of_node, pll->name, "ref", 0, mult, div, acc);
> + priv->cells->hws[pll->index] = hw;
> + if (IS_ERR(hw))
> + dev_warn(dev, "failed registering %s: %pe\n", pll->name, hw);
> + }
> +
> + BUG_ON(ARRAY_SIZE(div_resources) < data->div_count);
Can this be a static assert instead on the arrays these are based on?
Put some static_assert() near the match data macros.
> +
> + for (i = 0; i < data->div_count; i++) {
> + const struct eqc_div *div = &data->divs[i];
> + void __iomem *base = NULL;
> + struct clk_hw *parent;
> + unsigned int j;
> +
> + /*
> + * Multiple divider clocks can request the same resource. Store
> + * resource pointers during probe(). For each divider clock,
> + * check if previous clocks referenced the same resource name.
> + *
> + * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS.
> + */
> + for (j = 0; j < i; j++) {
> + if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) {
> + base = div_resources[j];
> + break;
> + }
> + }
> +
> + /* Resource is first encountered. */
> + if (!base) {
> + base = devm_platform_ioremap_resource_byname(pdev, div->resource_name);
> + if (IS_ERR(base)) {
> + dev_warn(dev, "failed to iomap resource for %s\n", div->name);
> + priv->cells->hws[div->index] = base;
> + continue;
> + }
> + }
I don't get this code at all. The driver should simply map the
resources because it knows that there's an io resource. I'll look at the
binding which is probably wrong and causing the driver to be written
this way.
> +
> + div_resources[i] = base;
> +
> + parent = priv->cells->hws[div->parent];
> + hw = clk_hw_register_divider_table_parent_hw(dev, div->name,
> + parent, 0, base, div->shift, div->width,
> + CLK_DIVIDER_EVEN_INTEGERS, NULL, NULL);
> + priv->cells->hws[div->index] = hw;
> + if (IS_ERR(hw))
> + dev_warn(dev, "failed registering %s: %pe\n",
> + div->name, hw);
> + }
> +
> + /* Clock provider has not been registered by eqc_init(). Do it now. */
> + if (data->early_pll_count == 0) {
> + /* When providing a single clock, require no cell. */
> + if (priv->cells->num == 1)
> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
> + priv->cells->hws);
> + else
> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> + priv->cells);
> + }
> +
> + return 0;
> +}
> +
> +/* Required early for GIC timer (pll-cpu) and UARTs (pll-per). */
> +static const struct eqc_pll eqc_eyeq5_early_plls[] = {
> + { .index = EQ5C_PLL_CPU, .name = "pll-cpu", .reg64 = 0x00, },
> + { .index = EQ5C_PLL_PER, .name = "pll-per", .reg64 = 0x30, },
> +};
> +
> +static const struct eqc_pll eqc_eyeq5_plls[] = {
> + { .index = EQ5C_PLL_VMP, .name = "pll-vmp", .reg64 = 0x08, },
> + { .index = EQ5C_PLL_PMA, .name = "pll-pma", .reg64 = 0x10, },
> + { .index = EQ5C_PLL_VDI, .name = "pll-vdi", .reg64 = 0x18, },
> + { .index = EQ5C_PLL_DDR0, .name = "pll-ddr0", .reg64 = 0x20, },
> + { .index = EQ5C_PLL_PCI, .name = "pll-pci", .reg64 = 0x28, },
> + { .index = EQ5C_PLL_PMAC, .name = "pll-pmac", .reg64 = 0x38, },
> + { .index = EQ5C_PLL_MPC, .name = "pll-mpc", .reg64 = 0x40, },
> + { .index = EQ5C_PLL_DDR1, .name = "pll-ddr1", .reg64 = 0x48, },
> +};
> +
> +static const struct eqc_div eqc_eyeq5_divs[] = {
> + {
> + .index = EQ5C_DIV_OSPI,
> + .name = "div-ospi",
> + .parent = EQ5C_PLL_PER,
> + .resource_name = "ospi",
> + .shift = 0,
> + .width = 4,
> + },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq5_match_data = {
> + .early_pll_count = ARRAY_SIZE(eqc_eyeq5_early_plls),
> + .early_plls = eqc_eyeq5_early_plls,
> +
> + .pll_count = ARRAY_SIZE(eqc_eyeq5_plls),
> + .plls = eqc_eyeq5_plls,
> +
> + .div_count = ARRAY_SIZE(eqc_eyeq5_divs),
> + .divs = eqc_eyeq5_divs,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6l_plls[] = {
> + { .index = EQ6LC_PLL_DDR, .name = "pll-ddr", .reg64 = 0x2C },
> + { .index = EQ6LC_PLL_CPU, .name = "pll-cpu", .reg64 = 0x34 }, /* also acc */
> + { .index = EQ6LC_PLL_PER, .name = "pll-per", .reg64 = 0x3C },
> + { .index = EQ6LC_PLL_VDI, .name = "pll-vdi", .reg64 = 0x44 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6l_match_data = {
> + .pll_count = ARRAY_SIZE(eqc_eyeq6l_plls),
> + .plls = eqc_eyeq6l_plls,
> +};
> +
> +/* Required early for GIC timer. */
> +static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = {
> + { .index = 0, .name = "pll-cpu", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_central_match_data = {
> + .early_pll_count = ARRAY_SIZE(eqc_eyeq6h_central_early_plls),
> + .early_plls = eqc_eyeq6h_central_early_plls,
> +};
> +
> +/* Required early for UART. */
> +static const struct eqc_pll eqc_eyeq6h_west_early_plls[] = {
> + { .index = 0, .name = "pll-west", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_west_match_data = {
> + .early_pll_count = ARRAY_SIZE(eqc_eyeq6h_west_early_plls),
> + .early_plls = eqc_eyeq6h_west_early_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_east_plls[] = {
> + { .index = 0, .name = "pll-east", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_east_match_data = {
> + .pll_count = ARRAY_SIZE(eqc_eyeq6h_east_plls),
> + .plls = eqc_eyeq6h_east_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_south_plls[] = {
> + { .index = EQ6HC_SOUTH_PLL_VDI, .name = "pll-vdi", .reg64 = 0x00 },
> + { .index = EQ6HC_SOUTH_PLL_PCIE, .name = "pll-pcie", .reg64 = 0x08 },
> + { .index = EQ6HC_SOUTH_PLL_PER, .name = "pll-per", .reg64 = 0x10 },
> + { .index = EQ6HC_SOUTH_PLL_ISP, .name = "pll-isp", .reg64 = 0x18 },
> +};
> +
> +static const struct eqc_div eqc_eyeq6h_south_divs[] = {
> + {
> + .index = EQ6HC_SOUTH_DIV_EMMC,
> + .name = "div-emmc",
> + .parent = EQ6HC_SOUTH_PLL_PER,
> + .resource_name = "emmc",
> + .shift = 4,
> + .width = 4,
> + },
> + {
> + .index = EQ6HC_SOUTH_DIV_OSPI_REF,
> + .name = "div-ospi-ref",
> + .parent = EQ6HC_SOUTH_PLL_PER,
> + .resource_name = "ospi",
> + .shift = 4,
> + .width = 4,
> + },
> + {
> + .index = EQ6HC_SOUTH_DIV_OSPI_SYS,
> + .name = "div-ospi-sys",
> + .parent = EQ6HC_SOUTH_PLL_PER,
> + .resource_name = "ospi",
> + .shift = 8,
> + .width = 1,
> + },
> + {
> + .index = EQ6HC_SOUTH_DIV_TSU,
> + .name = "div-tsu",
> + .parent = EQ6HC_SOUTH_PLL_PCIE,
> + .resource_name = "tsu",
> + .shift = 4,
> + .width = 8,
> + },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_south_match_data = {
> + .pll_count = ARRAY_SIZE(eqc_eyeq6h_south_plls),
> + .plls = eqc_eyeq6h_south_plls,
> +
> + .div_count = ARRAY_SIZE(eqc_eyeq6h_south_divs),
> + .divs = eqc_eyeq6h_south_divs,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_ddr0_plls[] = {
> + { .index = 0, .name = "pll-ddr0", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_ddr0_match_data = {
> + .pll_count = ARRAY_SIZE(eqc_eyeq6h_ddr0_plls),
> + .plls = eqc_eyeq6h_ddr0_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_ddr1_plls[] = {
> + { .index = 0, .name = "pll-ddr1", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_ddr1_match_data = {
> + .pll_count = ARRAY_SIZE(eqc_eyeq6h_ddr1_plls),
> + .plls = eqc_eyeq6h_ddr1_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_acc_plls[] = {
> + { .index = EQ6HC_ACC_PLL_XNN, .name = "pll-xnn", .reg64 = 0x00 },
> + { .index = EQ6HC_ACC_PLL_VMP, .name = "pll-vmp", .reg64 = 0x10 },
> + { .index = EQ6HC_ACC_PLL_PMA, .name = "pll-pma", .reg64 = 0x1C },
> + { .index = EQ6HC_ACC_PLL_MPC, .name = "pll-mpc", .reg64 = 0x28 },
> + { .index = EQ6HC_ACC_PLL_NOC, .name = "pll-noc", .reg64 = 0x30 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_acc_match_data = {
> + .pll_count = ARRAY_SIZE(eqc_eyeq6h_acc_plls),
> + .plls = eqc_eyeq6h_acc_plls,
> +};
> +
> +static const struct of_device_id eqc_match_table[] = {
> + { .compatible = "mobileye,eyeq5-clk", .data = &eqc_eyeq5_match_data },
> + { .compatible = "mobileye,eyeq6l-clk", .data = &eqc_eyeq6l_match_data },
> + { .compatible = "mobileye,eyeq6h-central-clk", .data = &eqc_eyeq6h_central_match_data },
> + { .compatible = "mobileye,eyeq6h-west-clk", .data = &eqc_eyeq6h_west_match_data },
> + { .compatible = "mobileye,eyeq6h-east-clk", .data = &eqc_eyeq6h_east_match_data },
> + { .compatible = "mobileye,eyeq6h-south-clk", .data = &eqc_eyeq6h_south_match_data },
> + { .compatible = "mobileye,eyeq6h-ddr0-clk", .data = &eqc_eyeq6h_ddr0_match_data },
> + { .compatible = "mobileye,eyeq6h-ddr1-clk", .data = &eqc_eyeq6h_ddr1_match_data },
> + { .compatible = "mobileye,eyeq6h-acc-clk", .data = &eqc_eyeq6h_acc_match_data },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, eqc_match_table);
> +
> +static struct platform_driver eqc_driver = {
> + .probe = eqc_probe,
> + .driver = {
> + .name = "clk-eyeq",
> + .of_match_table = eqc_match_table,
> + },
> +};
> +builtin_platform_driver(eqc_driver);
> +
> +static void __init eqc_init(struct device_node *np)
> +{
> + const struct eqc_match_data *data;
> + unsigned int nb_clks = 0;
> + struct eqc_priv *priv;
> + unsigned int i;
> + int ret;
> +
> + data = of_match_node(eqc_match_table, np)->data;
> +
> + /* No reason to early init this clock provider. Do it at probe. */
> + if (data->early_pll_count == 0)
You can have a different match table for this function then.
> + return;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + priv->np = np;
> + priv->data = data;
> +
> + nb_clks = eqc_compute_clock_count(data);
> + priv->cells = kzalloc(struct_size(priv->cells, hws, nb_clks), GFP_KERNEL);
> + if (!priv->cells) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + priv->cells->num = nb_clks;
> +
> + /*
> + * Mark non-early clocks as deferred; they'll be registered at platform
> + * device probe.
> + */
> + for (i = 0; i < data->pll_count; i++)
> + priv->cells->hws[data->plls[i].index] = ERR_PTR(-EPROBE_DEFER);
> + for (i = 0; i < data->div_count; i++)
> + priv->cells->hws[data->divs[i].index] = ERR_PTR(-EPROBE_DEFER);
> +
> + /*
> + * We expect named resources if divider clocks are present.
> + * Else, we only expect one resource.
> + */
Please avoid named resources. They give the false sense of hope that the
binding can re-order the reg property when that can't be done. Instead,
just index and know which index to use in the driver.
^ permalink raw reply
* Re: [PATCH 01/11] dt-bindings: soc: mobileye: add EyeQ5 OLB system controller
From: Stephen Boyd @ 2024-04-11 3:29 UTC (permalink / raw)
To: Conor Dooley, Krzysztof Kozlowski, Linus Walleij,
Michael Turquette, Philipp Zabel, Rob Herring, Théo Lebrun
Cc: linux-mips, devicetree, linux-kernel, linux-clk, linux-gpio,
Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
Tawfik Bayouk, Théo Lebrun
In-Reply-To: <20240410-mbly-olb-v1-1-335e496d7be3@bootlin.com>
Quoting Théo Lebrun (2024-04-10 10:12:30)
> diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
> new file mode 100644
> index 000000000000..c4e33a167fab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
> @@ -0,0 +1,125 @@
[..]
> +
> + reg:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 1
> +
> + ranges: true
> +
> +patternProperties:
> + '^clock-controller@[0-9a-f]+$':
> + $ref: /schemas/clock/mobileye,eyeq5-clk.yaml#
> +
> + '^reset-controller@[0-9a-f]+$':
> + $ref: /schemas/reset/mobileye,eyeq5-reset.yaml#
> +
> + '^pinctrl@[0-9a-f]+$':
> + $ref: /schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
Yep, there shouldn't be subnodes for these. Instead, olb should have
#clock-cells, #reset-cells, etc. and the driver registers auxiliary
devices for each driver like the clk driver, reset driver, pinctrl
driver. Then we don't need syscon or simple-mfd and random other drivers
can't use the regmap.
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Stephen Boyd @ 2024-04-11 3:35 UTC (permalink / raw)
To: Lorenzo Bianconi, linux-clk
Cc: mturquette, linux-arm-kernel, robh+dt, krzysztof.kozlowski+dt,
conor+dt, nbd, john, devicetree, dd, catalin.marinas, will,
upstream, lorenzo.bianconi83, angelogioacchino.delregno
In-Reply-To: <99734deb28889e685a764da94418f68b55ee3bdc.1712399981.git.lorenzo@kernel.org>
Quoting Lorenzo Bianconi (2024-04-06 03:43:41)
> Introduce Airoha EN7581 entry in Airoha EN7523 clock binding
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v2 3/4] clk: en7523: Add en_clk_soc_data data structure
From: Stephen Boyd @ 2024-04-11 3:36 UTC (permalink / raw)
To: Lorenzo Bianconi, linux-clk
Cc: mturquette, linux-arm-kernel, robh+dt, krzysztof.kozlowski+dt,
conor+dt, nbd, john, devicetree, dd, catalin.marinas, will,
upstream, lorenzo.bianconi83, angelogioacchino.delregno
In-Reply-To: <562a0da8d7874a02a324687c152c87a1549924bd.1712399981.git.lorenzo@kernel.org>
Quoting Lorenzo Bianconi (2024-04-06 03:43:43)
> Introduce en_clk_soc_data data structure in order to define multiple
> clk_ops for each supported SoC. This is a preliminary patch to
> introduce EN7581 clock support.
>
> Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
Applied to clk-next
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox