Message ID | 20190526091326.31141-1-paul.walmsley@sifive.com |
---|---|
State | Superseded |
Delegated to: | Andes |
Headers | show |
Series | [U-Boot] clk: sifive: fu540-prci: align primary DT match string to the DT bindings | expand |
On 5/26/19 2:13 AM, Paul Walmsley wrote: > The U-Boot PRCI driver for the SiFive FU540 uses an out-of-date DT > binding string, since the U-boot PRCI driver was upstreamed before the > mainline Linux kernel PRCI driver was finished. This means that the > U-Boot PRCI driver won't probe when used with a DT file that is > aligned to the DT bindings and the driver in the Linux kernel: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt#n7 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/sifive/fu540-prci.c#n610 > > Fix by aligning the U-Boot DT match string to the string that's used > in the upstream DT bindings and the Linux kernel driver. > > > Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com> > Signed-off-by: Paul Walmsley <paul@pwsan.com> > Cc: Anup Patel <Anup.Patel@wdc.com> > Cc: Atish Patra <atish.patra@wdc.com> > Cc: Alexander Graf <agraf@suse.de> > --- > drivers/clk/sifive/fu540-prci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > index 2d47ebc6b1..d79d1a5351 100644 > --- a/drivers/clk/sifive/fu540-prci.c > +++ b/drivers/clk/sifive/fu540-prci.c > @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > }; > > static const struct udevice_id sifive_fu540_prci_ids[] = { > - { .compatible = "sifive,fu540-c000-prci0" }, > + { .compatible = "sifive,fu540-c000-prci" }, > { .compatible = "sifive,aloeprci0" }, > { } > }; > Reviewed-by: Atish Patra <atish.patra@wdc.com>
On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > The U-Boot PRCI driver for the SiFive FU540 uses an out-of-date DT > binding string, since the U-boot PRCI driver was upstreamed before the nits: U-boot -> U-Boot > mainline Linux kernel PRCI driver was finished. This means that the > U-Boot PRCI driver won't probe when used with a DT file that is > aligned to the DT bindings and the driver in the Linux kernel: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt#n7 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/sifive/fu540-prci.c#n610 > > Fix by aligning the U-Boot DT match string to the string that's used > in the upstream DT bindings and the Linux kernel driver. > > > Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com> > Signed-off-by: Paul Walmsley <paul@pwsan.com> > Cc: Anup Patel <Anup.Patel@wdc.com> > Cc: Atish Patra <atish.patra@wdc.com> > Cc: Alexander Graf <agraf@suse.de> > --- > drivers/clk/sifive/fu540-prci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > index 2d47ebc6b1..d79d1a5351 100644 > --- a/drivers/clk/sifive/fu540-prci.c > +++ b/drivers/clk/sifive/fu540-prci.c > @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > }; > > static const struct udevice_id sifive_fu540_prci_ids[] = { > - { .compatible = "sifive,fu540-c000-prci0" }, > + { .compatible = "sifive,fu540-c000-prci" }, Can we keep the previous compatible string for compatibility reason? U-Boot is now reusing the DT that FSBL passes. Changing the name here means we need re-flash an updated FSBL? > { .compatible = "sifive,aloeprci0" }, > { } > }; > -- Regards, Bin
On 5/29/19 6:19 PM, Bin Meng wrote: > On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: >> >> The U-Boot PRCI driver for the SiFive FU540 uses an out-of-date DT >> binding string, since the U-boot PRCI driver was upstreamed before the > > nits: U-boot -> U-Boot > >> mainline Linux kernel PRCI driver was finished. This means that the >> U-Boot PRCI driver won't probe when used with a DT file that is >> aligned to the DT bindings and the driver in the Linux kernel: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt#n7 >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/sifive/fu540-prci.c#n610 >> >> Fix by aligning the U-Boot DT match string to the string that's used >> in the upstream DT bindings and the Linux kernel driver. >> >> >> Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com> >> Signed-off-by: Paul Walmsley <paul@pwsan.com> >> Cc: Anup Patel <Anup.Patel@wdc.com> >> Cc: Atish Patra <atish.patra@wdc.com> >> Cc: Alexander Graf <agraf@suse.de> >> --- >> drivers/clk/sifive/fu540-prci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c >> index 2d47ebc6b1..d79d1a5351 100644 >> --- a/drivers/clk/sifive/fu540-prci.c >> +++ b/drivers/clk/sifive/fu540-prci.c >> @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { >> }; >> >> static const struct udevice_id sifive_fu540_prci_ids[] = { >> - { .compatible = "sifive,fu540-c000-prci0" }, >> + { .compatible = "sifive,fu540-c000-prci" }, > > Can we keep the previous compatible string for compatibility reason? > U-Boot is now reusing the DT that FSBL passes. Changing the name here > means we need re-flash an updated FSBL? Unfortunately yes. However, you can also use OpenSBI/BBL to use the updated DT instead of DT from FSBL. OpenSBI method: Just use the additional argument during OpenSBI compilation. FW_PAYLOAD_FDT_PATH=<linux kernel source>/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dtb BBL: Paul has a working branch. Details are here http://lists.infradead.org/pipermail/linux-riscv/2019-May/004685.html > >> { .compatible = "sifive,aloeprci0" }, >> { } >> }; >> -- > > Regards, > Bin > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot >
On Thu, May 30, 2019 at 1:06 PM Atish Patra <atish.patra@wdc.com> wrote: > > On 5/29/19 6:19 PM, Bin Meng wrote: > > On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > >> > >> The U-Boot PRCI driver for the SiFive FU540 uses an out-of-date DT > >> binding string, since the U-boot PRCI driver was upstreamed before the > > > > nits: U-boot -> U-Boot > > > >> mainline Linux kernel PRCI driver was finished. This means that the > >> U-Boot PRCI driver won't probe when used with a DT file that is > >> aligned to the DT bindings and the driver in the Linux kernel: > >> > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt#n7 > >> > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/sifive/fu540-prci.c#n610 > >> > >> Fix by aligning the U-Boot DT match string to the string that's used > >> in the upstream DT bindings and the Linux kernel driver. > >> > >> > >> Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com> > >> Signed-off-by: Paul Walmsley <paul@pwsan.com> > >> Cc: Anup Patel <Anup.Patel@wdc.com> > >> Cc: Atish Patra <atish.patra@wdc.com> > >> Cc: Alexander Graf <agraf@suse.de> > >> --- > >> drivers/clk/sifive/fu540-prci.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > >> index 2d47ebc6b1..d79d1a5351 100644 > >> --- a/drivers/clk/sifive/fu540-prci.c > >> +++ b/drivers/clk/sifive/fu540-prci.c > >> @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > >> }; > >> > >> static const struct udevice_id sifive_fu540_prci_ids[] = { > >> - { .compatible = "sifive,fu540-c000-prci0" }, > >> + { .compatible = "sifive,fu540-c000-prci" }, > > > > Can we keep the previous compatible string for compatibility reason? > > U-Boot is now reusing the DT that FSBL passes. Changing the name here > > means we need re-flash an updated FSBL? > > Unfortunately yes. However, you can also use OpenSBI/BBL to use the > updated DT instead of DT from FSBL. > > OpenSBI method: Just use the additional argument during OpenSBI compilation. > > FW_PAYLOAD_FDT_PATH=<linux kernel > source>/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dtb OK, please update the doc/README.sifive-fu540, to mention an updated FDT needs to be provided to OpenSBI, if we remove the old compatible string in the U-Boot PRCI driver. > > BBL: Paul has a working branch. Details are here > > http://lists.infradead.org/pipermail/linux-riscv/2019-May/004685.html > Regards, Bin
On 5/29/19 10:48 PM, Bin Meng wrote: > On Thu, May 30, 2019 at 1:06 PM Atish Patra <atish.patra@wdc.com> wrote: >> >> On 5/29/19 6:19 PM, Bin Meng wrote: >>> On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: >>>> >>>> The U-Boot PRCI driver for the SiFive FU540 uses an out-of-date DT >>>> binding string, since the U-boot PRCI driver was upstreamed before the >>> >>> nits: U-boot -> U-Boot >>> >>>> mainline Linux kernel PRCI driver was finished. This means that the >>>> U-Boot PRCI driver won't probe when used with a DT file that is >>>> aligned to the DT bindings and the driver in the Linux kernel: >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt#n7 >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/sifive/fu540-prci.c#n610 >>>> >>>> Fix by aligning the U-Boot DT match string to the string that's used >>>> in the upstream DT bindings and the Linux kernel driver. >>>> >>>> >>>> Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com> >>>> Signed-off-by: Paul Walmsley <paul@pwsan.com> >>>> Cc: Anup Patel <Anup.Patel@wdc.com> >>>> Cc: Atish Patra <atish.patra@wdc.com> >>>> Cc: Alexander Graf <agraf@suse.de> >>>> --- >>>> drivers/clk/sifive/fu540-prci.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c >>>> index 2d47ebc6b1..d79d1a5351 100644 >>>> --- a/drivers/clk/sifive/fu540-prci.c >>>> +++ b/drivers/clk/sifive/fu540-prci.c >>>> @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { >>>> }; >>>> >>>> static const struct udevice_id sifive_fu540_prci_ids[] = { >>>> - { .compatible = "sifive,fu540-c000-prci0" }, >>>> + { .compatible = "sifive,fu540-c000-prci" }, >>> >>> Can we keep the previous compatible string for compatibility reason? >>> U-Boot is now reusing the DT that FSBL passes. Changing the name here >>> means we need re-flash an updated FSBL? >> >> Unfortunately yes. However, you can also use OpenSBI/BBL to use the >> updated DT instead of DT from FSBL. >> >> OpenSBI method: Just use the additional argument during OpenSBI compilation. >> >> FW_PAYLOAD_FDT_PATH=<linux kernel >> source>/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dtb > > OK, please update the doc/README.sifive-fu540, to mention an updated > FDT needs to be provided to OpenSBI, if we remove the old compatible > string in the U-Boot PRCI driver. > Sure. I think we need to update it anyways once the clock driver is fixed as per new DT. I have not looked into details but I was not able to boot U-Boot only with this patch after using the new DT from kernel. This is expected as compatible string is not the only change as described in Linux mailing list[1]. [1] http://lists.infradead.org/pipermail/linux-riscv/2019-April/004259.html >> >> BBL: Paul has a working branch. Details are here >> >> http://lists.infradead.org/pipermail/linux-riscv/2019-May/004685.html >> > > Regards, > Bin >
On Thu, 30 May 2019, Bin Meng wrote: > On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > > index 2d47ebc6b1..d79d1a5351 100644 > > --- a/drivers/clk/sifive/fu540-prci.c > > +++ b/drivers/clk/sifive/fu540-prci.c > > @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > > }; > > > > static const struct udevice_id sifive_fu540_prci_ids[] = { > > - { .compatible = "sifive,fu540-c000-prci0" }, > > + { .compatible = "sifive,fu540-c000-prci" }, > > Can we keep the previous compatible string for compatibility reason? > U-Boot is now reusing the DT that FSBL passes. Are there any FSBLs that pass "sifive,fu540-c000-prci0" ? I am not aware of any. SiFive FSBLs have only ever used "sifive,aloeprci0" (or "sifive,ux00prci0") and those will soon be deprecated. It would be good if the U-Boot maintainers would reject any DT compatible strings that haven't been committed upstream into the Linux kernel DT binding repository. Otherwise I foresee this kind of mess increasing if others decide to invent their own compatible strings. - Paul
Hello Bin, On Fri, 31 May 2019, Paul Walmsley wrote: > On Thu, 30 May 2019, Bin Meng wrote: > > > On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > > > index 2d47ebc6b1..d79d1a5351 100644 > > > --- a/drivers/clk/sifive/fu540-prci.c > > > +++ b/drivers/clk/sifive/fu540-prci.c > > > @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > > > }; > > > > > > static const struct udevice_id sifive_fu540_prci_ids[] = { > > > - { .compatible = "sifive,fu540-c000-prci0" }, > > > + { .compatible = "sifive,fu540-c000-prci" }, > > > > Can we keep the previous compatible string for compatibility reason? > > U-Boot is now reusing the DT that FSBL passes. > > Are there any FSBLs that pass "sifive,fu540-c000-prci0" ? I am not aware > of any. > > SiFive FSBLs have only ever used "sifive,aloeprci0" (or > "sifive,ux00prci0") and those will soon be deprecated. Just checking in again on this patch. Do you still need "sifive,fu540-c000-prci0" to be preserved, even though it should be unused? Or is the original patch OK for you? - Paul
Hi Paul, On Fri, Jun 7, 2019 at 1:45 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > Hello Bin, > > On Fri, 31 May 2019, Paul Walmsley wrote: > > > On Thu, 30 May 2019, Bin Meng wrote: > > > > > On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > > > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > > > > index 2d47ebc6b1..d79d1a5351 100644 > > > > --- a/drivers/clk/sifive/fu540-prci.c > > > > +++ b/drivers/clk/sifive/fu540-prci.c > > > > @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > > > > }; > > > > > > > > static const struct udevice_id sifive_fu540_prci_ids[] = { > > > > - { .compatible = "sifive,fu540-c000-prci0" }, > > > > + { .compatible = "sifive,fu540-c000-prci" }, > > > > > > Can we keep the previous compatible string for compatibility reason? > > > U-Boot is now reusing the DT that FSBL passes. > > > > Are there any FSBLs that pass "sifive,fu540-c000-prci0" ? I am not aware > > of any. > > > > SiFive FSBLs have only ever used "sifive,aloeprci0" (or > > "sifive,ux00prci0") and those will soon be deprecated. > > Just checking in again on this patch. Do you still need > "sifive,fu540-c000-prci0" to be preserved, even though it should be > unused? Or is the original patch OK for you? The original patch looks OK to me. Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Regards, Bin
Hi Rick and other U-Boot folks, On Fri, 7 Jun 2019, Bin Meng wrote: > Hi Paul, > > On Fri, Jun 7, 2019 at 1:45 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > Hello Bin, > > > > On Fri, 31 May 2019, Paul Walmsley wrote: > > > > > On Thu, 30 May 2019, Bin Meng wrote: > > > > > > > On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > > > > > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > > > > > index 2d47ebc6b1..d79d1a5351 100644 > > > > > --- a/drivers/clk/sifive/fu540-prci.c > > > > > +++ b/drivers/clk/sifive/fu540-prci.c > > > > > @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > > > > > }; > > > > > > > > > > static const struct udevice_id sifive_fu540_prci_ids[] = { > > > > > - { .compatible = "sifive,fu540-c000-prci0" }, > > > > > + { .compatible = "sifive,fu540-c000-prci" }, > > > > > > > > Can we keep the previous compatible string for compatibility reason? > > > > U-Boot is now reusing the DT that FSBL passes. > > > > > > Are there any FSBLs that pass "sifive,fu540-c000-prci0" ? I am not aware > > > of any. > > > > > > SiFive FSBLs have only ever used "sifive,aloeprci0" (or > > > "sifive,ux00prci0") and those will soon be deprecated. > > > > Just checking in again on this patch. Do you still need > > "sifive,fu540-c000-prci0" to be preserved, even though it should be > > unused? Or is the original patch OK for you? > > The original patch looks OK to me. > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Any comments on this patch? Anything else that needs to be done before it can be merged? - Paul
Hi Paul > Hi Rick and other U-Boot folks, > > On Fri, 7 Jun 2019, Bin Meng wrote: > > > Hi Paul, > > > > On Fri, Jun 7, 2019 at 1:45 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > > > Hello Bin, > > > > > > On Fri, 31 May 2019, Paul Walmsley wrote: > > > > > > > On Thu, 30 May 2019, Bin Meng wrote: > > > > > > > > > On Sun, May 26, 2019 at 5:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > > > > > > > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > > > > > > index 2d47ebc6b1..d79d1a5351 100644 > > > > > > --- a/drivers/clk/sifive/fu540-prci.c > > > > > > +++ b/drivers/clk/sifive/fu540-prci.c > > > > > > @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { > > > > > > }; > > > > > > > > > > > > static const struct udevice_id sifive_fu540_prci_ids[] = { > > > > > > - { .compatible = "sifive,fu540-c000-prci0" }, > > > > > > + { .compatible = "sifive,fu540-c000-prci" }, > > > > > > > > > > Can we keep the previous compatible string for compatibility reason? > > > > > U-Boot is now reusing the DT that FSBL passes. > > > > > > > > Are there any FSBLs that pass "sifive,fu540-c000-prci0" ? I am not aware > > > > of any. > > > > > > > > SiFive FSBLs have only ever used "sifive,aloeprci0" (or > > > > "sifive,ux00prci0") and those will soon be deprecated. > > > > > > Just checking in again on this patch. Do you still need > > > "sifive,fu540-c000-prci0" to be preserved, even though it should be > > > unused? Or is the original patch OK for you? > > > > The original patch looks OK to me. > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > Any comments on this patch? Anything else that needs to be done before it > can be merged? > This modification has been done by Anup's patch: [PATCH v9 4/9] clk: sifive: Sync-up main driver with upstream Linux. Thanks Rick > > - Paul
diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c index 2d47ebc6b1..d79d1a5351 100644 --- a/drivers/clk/sifive/fu540-prci.c +++ b/drivers/clk/sifive/fu540-prci.c @@ -589,7 +589,7 @@ static struct clk_ops sifive_fu540_prci_ops = { }; static const struct udevice_id sifive_fu540_prci_ids[] = { - { .compatible = "sifive,fu540-c000-prci0" }, + { .compatible = "sifive,fu540-c000-prci" }, { .compatible = "sifive,aloeprci0" }, { } };