diff mbox series

[v6,7/7] arm64: dts: renesas: r8a779f0: spider-cpu: Enable UFS device

Message ID 20220603110524.1997825-8-yoshihiro.shimoda.uh@renesas.com
State New
Headers show
Series treewide: ufs: Add support for Renesas R-Car UFS controller | expand

Commit Message

Yoshihiro Shimoda June 3, 2022, 11:05 a.m. UTC 
Enable UFS device for R-Car S4-8 Spider CPU board.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a779f0-spider-cpu.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Geert Uytterhoeven June 14, 2022, 8:06 a.m. UTC | #1 
Hi Shimoda-san,

On Fri, Jun 3, 2022 at 1:05 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Enable UFS device for R-Car S4-8 Spider CPU board.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/arm64/boot/dts/renesas/r8a779f0-spider-cpu.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779f0-spider-cpu.dtsi
> @@ -82,3 +82,11 @@ &scif3 {
>  &scif_clk {
>         clock-frequency = <24000000>;
>  };
> +
> +&ufs {
> +       status = "okay";
> +};
> +
> +&ufs30_clk {
> +       clock-frequency = <38400000>;
> +};

Given this relies on either the boot loader setting up ufs30_clk,
like is usually done for the PCIe bus clock, or on adding a proper
clock driver to Linux, I think this patch should be postponed.

Or perhaps the latest firmware stack has fixed the issue?
Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Wolfram Sang June 21, 2022, 3:14 p.m. UTC | #2 
Hi Shimoda-san, Geert,

On Fri, Jun 03, 2022 at 08:05:24PM +0900, Yoshihiro Shimoda wrote:
> Enable UFS device for R-Car S4-8 Spider CPU board.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

With my firmware, the manual setup of the clock in the bootloader is
still needed. So, yes, I agree we should wait with this patch until we
have a better way to deal with the clock.

Other than that, the patches give me SCSI disks I can work with
(partition, read, write). There are a few initial errors, though:

[    0.449917] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03 for idn 1 failed, index 0, err = 253
[    0.452035] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03 for idn 1 failed, index 0, err = 253
[    0.453859] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03 for idn 1 failed, index 0, err = 253
[    0.453874] ufshcd-renesas e6860000.ufs: ufshcd_query_attr_retry: query attribute, idn 1, failed with error 253 after 3 retires

(A patch for the typo in the last line has already been sent)

But after that, everything looks fine on first testing. So, for the
patches:

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Happy hacking!
Avri Altman June 22, 2022, 6:32 a.m. UTC | #3 
> Hi Shimoda-san, Geert,
> 
> On Fri, Jun 03, 2022 at 08:05:24PM +0900, Yoshihiro Shimoda wrote:
> > Enable UFS device for R-Car S4-8 Spider CPU board.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> With my firmware, the manual setup of the clock in the bootloader is still
> needed. So, yes, I agree we should wait with this patch until we have a better
> way to deal with the clock.
> 
> Other than that, the patches give me SCSI disks I can work with (partition,
> read, write). There are a few initial errors, though:
> 
> [    0.449917] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03
> for idn 1 failed, index 0, err = 253
> [    0.452035] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03
> for idn 1 failed, index 0, err = 253
> [    0.453859] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03
> for idn 1 failed, index 0, err = 253
> [    0.453874] ufshcd-renesas e6860000.ufs: ufshcd_query_attr_retry: query
> attribute, idn 1, failed with error 253 after 3 retires
Should be interesting to find out who is trying to read an undefined (reserved) attribute in your system.

Thanks,
Avri
> 
> (A patch for the typo in the last line has already been sent)
> 
> But after that, everything looks fine on first testing. So, for the
> patches:
> 
> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Happy hacking!
Yoshihiro Shimoda June 22, 2022, 8:11 a.m. UTC | #4 
Hi Wolfram-san,

> From: Wolfram Sang, Sent: Wednesday, June 22, 2022 12:14 AM
> 
> Hi Shimoda-san, Geert,
> 
> On Fri, Jun 03, 2022 at 08:05:24PM +0900, Yoshihiro Shimoda wrote:
> > Enable UFS device for R-Car S4-8 Spider CPU board.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> With my firmware, the manual setup of the clock in the bootloader is
> still needed. So, yes, I agree we should wait with this patch until we
> have a better way to deal with the clock.
> 
> Other than that, the patches give me SCSI disks I can work with
> (partition, read, write). There are a few initial errors, though:
> 
> [    0.449917] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03 for idn 1 failed, index 0, err = 253
> [    0.452035] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03 for idn 1 failed, index 0, err = 253
> [    0.453859] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03 for idn 1 failed, index 0, err = 253
> [    0.453874] ufshcd-renesas e6860000.ufs: ufshcd_query_attr_retry: query attribute, idn 1, failed with error 253 after
> 3 retires

Hmm, my environment [1] could not reproduce this error messages. 

[1]
based on renesas-drivers-2022-05-24-v5.18 which I made the patches.
Perhaps, should I test on the latest kernel?
Which kernel version did you test?

> (A patch for the typo in the last line has already been sent)

Thank you for sending a patch!

> But after that, everything looks fine on first testing. So, for the
> patches:
> 
> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thank you for testing!

Best regards,
Yoshihiro Shimoda

> Happy hacking!
Wolfram Sang June 22, 2022, 8:22 a.m. UTC | #5 
Hi Shimoda-san,

> Hmm, my environment [1] could not reproduce this error messages. 

Interesting. I will add some debug output to provide more information.

> based on renesas-drivers-2022-05-24-v5.18 which I made the patches.
> Perhaps, should I test on the latest kernel?
> Which kernel version did you test?

renesas-drivers-2022-06-07-v5.19-rc1 with my Thermal, HSCIF, and your
UFS patches on top. I pushed it here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/s4/ufs-experimental

I'll be back soon,

   Wolfram
Yoshihiro Shimoda June 22, 2022, 8:28 a.m. UTC | #6 
Hi Wolfram-san,

> From: Wolfram Sang, Sent: Wednesday, June 22, 2022 5:23 PM
> 
> Hi Shimoda-san,
> 
> > Hmm, my environment [1] could not reproduce this error messages.
> 
> Interesting. I will add some debug output to provide more information.

Thanks!

> > based on renesas-drivers-2022-05-24-v5.18 which I made the patches.
> > Perhaps, should I test on the latest kernel?
> > Which kernel version did you test?
> 
> renesas-drivers-2022-06-07-v5.19-rc1 with my Thermal, HSCIF, and your
> UFS patches on top. I pushed it here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/s4/ufs-experimental

Thank you for sharing your branch. I'll check this tomorrow.

> I'll be back soon,

I'm afraid, but I'll be out-of-office today after I sent this email.
So, no rush is needed :)

Best regards,
Yoshihiro Shimoda

>    Wolfram
Wolfram Sang June 22, 2022, 8:38 a.m. UTC | #7 
Hi Avri, all,

> > [    0.449917] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03
> > for idn 1 failed, index 0, err = 253
> > [    0.452035] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03
> > for idn 1 failed, index 0, err = 253
> > [    0.453859] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode 0x03
> > for idn 1 failed, index 0, err = 253
> > [    0.453874] ufshcd-renesas e6860000.ufs: ufshcd_query_attr_retry: query
> > attribute, idn 1, failed with error 253 after 3 retires
> Should be interesting to find out who is trying to read an undefined (reserved) attribute in your system.

So, the call trace is:

[    0.455361] Call trace:
[    0.455521]  ufshcd_query_attr_retry+0x68/0xb0
[    0.455808]  ufshpb_get_dev_info+0x80/0x110
[    0.456083]  ufshcd_probe_hba+0xce0/0x10d0
[    0.456349]  ufshcd_async_scan+0x34/0x310
[    0.456609]  async_run_entry_fn+0x34/0x130
[    0.456873]  process_one_work+0x1e4/0x434
[    0.457136]  worker_thread+0x174/0x4dc
[    0.457379]  kthread+0xdc/0xe0
[    0.457580]  ret_from_fork+0x10/0x20

which leads me to this call in ufshpb_get_dev_info():

2622         ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
2623                 QUERY_ATTR_IDN_MAX_HPB_SINGLE_CMD, 0, 0, &max_single_cmd);

And from here on, I miss the UFS experience to debug further. But I will
happily provide more information if people give me pointers.

All the best,

   Wolfram
Avri Altman June 22, 2022, 9:42 a.m. UTC | #8 
> Hi Avri, all,
> 
> > > [    0.449917] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> 0x03
> > > for idn 1 failed, index 0, err = 253
> > > [    0.452035] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> 0x03
> > > for idn 1 failed, index 0, err = 253
> > > [    0.453859] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> 0x03
> > > for idn 1 failed, index 0, err = 253
> > > [    0.453874] ufshcd-renesas e6860000.ufs: ufshcd_query_attr_retry:
> query
> > > attribute, idn 1, failed with error 253 after 3 retires
> > Should be interesting to find out who is trying to read an undefined
> (reserved) attribute in your system.
> 
> So, the call trace is:
> 
> [    0.455361] Call trace:
> [    0.455521]  ufshcd_query_attr_retry+0x68/0xb0
> [    0.455808]  ufshpb_get_dev_info+0x80/0x110
> [    0.456083]  ufshcd_probe_hba+0xce0/0x10d0
> [    0.456349]  ufshcd_async_scan+0x34/0x310
> [    0.456609]  async_run_entry_fn+0x34/0x130
> [    0.456873]  process_one_work+0x1e4/0x434
> [    0.457136]  worker_thread+0x174/0x4dc
> [    0.457379]  kthread+0xdc/0xe0
> [    0.457580]  ret_from_fork+0x10/0x20
> 
> which leads me to this call in ufshpb_get_dev_info():
> 
> 2622         ret = ufshcd_query_attr_retry(hba,
> UPIU_QUERY_OPCODE_READ_ATTR,
> 2623                 QUERY_ATTR_IDN_MAX_HPB_SINGLE_CMD, 0, 0,
> &max_single_cmd);
> 
> And from here on, I miss the UFS experience to debug further. But I will
> happily provide more information if people give me pointers.
Ah ok.
That's cool - HPB is enable on your platform.
For some reason JEDEC didn't merge the HPB amendment into UFS4.0 - and I forgot all about that attribute.

Thanks,
Avri

> 
> All the best,
> 
>    Wolfram
Avri Altman June 22, 2022, 9:47 a.m. UTC | #9 
> > Hi Avri, all,
> >
> > > > [    0.449917] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> > 0x03
> > > > for idn 1 failed, index 0, err = 253
> > > > [    0.452035] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> > 0x03
> > > > for idn 1 failed, index 0, err = 253
> > > > [    0.453859] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> > 0x03
> > > > for idn 1 failed, index 0, err = 253
> > > > [    0.453874] ufshcd-renesas e6860000.ufs: ufshcd_query_attr_retry:
> > query
> > > > attribute, idn 1, failed with error 253 after 3 retires
> > > Should be interesting to find out who is trying to read an undefined
> > (reserved) attribute in your system.
> >
> > So, the call trace is:
> >
> > [    0.455361] Call trace:
> > [    0.455521]  ufshcd_query_attr_retry+0x68/0xb0
> > [    0.455808]  ufshpb_get_dev_info+0x80/0x110
> > [    0.456083]  ufshcd_probe_hba+0xce0/0x10d0
> > [    0.456349]  ufshcd_async_scan+0x34/0x310
> > [    0.456609]  async_run_entry_fn+0x34/0x130
> > [    0.456873]  process_one_work+0x1e4/0x434
> > [    0.457136]  worker_thread+0x174/0x4dc
> > [    0.457379]  kthread+0xdc/0xe0
> > [    0.457580]  ret_from_fork+0x10/0x20
> >
> > which leads me to this call in ufshpb_get_dev_info():
> >
> > 2622         ret = ufshcd_query_attr_retry(hba,
> > UPIU_QUERY_OPCODE_READ_ATTR,
> > 2623                 QUERY_ATTR_IDN_MAX_HPB_SINGLE_CMD, 0, 0,
> > &max_single_cmd);
> >
> > And from here on, I miss the UFS experience to debug further. But I
> > will happily provide more information if people give me pointers.
> Ah ok.
> That's cool - HPB is enable on your platform.
> For some reason JEDEC didn't merge the HPB amendment into UFS4.0 - and I
> forgot all about that attribute.
And the source of this error is that your device does not support HPB2.0,
Which is fine, because HPB2.0 support was removed a while ago.

Thanks,
Avri

> 
> Thanks,
> Avri
> 
> >
> > All the best,
> >
> >    Wolfram
Wolfram Sang June 22, 2022, 11:16 a.m. UTC | #10 
Hi Avri,

> > > which leads me to this call in ufshpb_get_dev_info():
> > >
> > > 2622         ret = ufshcd_query_attr_retry(hba,
> > > UPIU_QUERY_OPCODE_READ_ATTR,
> > > 2623                 QUERY_ATTR_IDN_MAX_HPB_SINGLE_CMD, 0, 0,
> > > &max_single_cmd);
> > >
> > > And from here on, I miss the UFS experience to debug further. But I
> > > will happily provide more information if people give me pointers.
> > Ah ok.
> > That's cool - HPB is enable on your platform.
> > For some reason JEDEC didn't merge the HPB amendment into UFS4.0 - and I
> > forgot all about that attribute.
> And the source of this error is that your device does not support HPB2.0,
> Which is fine, because HPB2.0 support was removed a while ago.

As I understand, the UFS core needs an update then? If you CC me on
patches, I will test them right away.

Happy hacking,

   Wolfram
Yoshihiro Shimoda June 23, 2022, 4:20 a.m. UTC | #11 
Hi Avri, Wolfram-san,

> From: Avri Altman, Sent: Wednesday, June 22, 2022 6:47 PM
> 
> > > Hi Avri, all,
> > >
> > > > > [    0.449917] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> > > 0x03
> > > > > for idn 1 failed, index 0, err = 253
> > > > > [    0.452035] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> > > 0x03
> > > > > for idn 1 failed, index 0, err = 253
> > > > > [    0.453859] ufshcd-renesas e6860000.ufs: ufshcd_query_attr: opcode
> > > 0x03
> > > > > for idn 1 failed, index 0, err = 253
> > > > > [    0.453874] ufshcd-renesas e6860000.ufs: ufshcd_query_attr_retry:
> > > query
> > > > > attribute, idn 1, failed with error 253 after 3 retires
> > > > Should be interesting to find out who is trying to read an undefined
> > > (reserved) attribute in your system.
> > >
> > > So, the call trace is:
> > >
> > > [    0.455361] Call trace:
> > > [    0.455521]  ufshcd_query_attr_retry+0x68/0xb0
> > > [    0.455808]  ufshpb_get_dev_info+0x80/0x110
> > > [    0.456083]  ufshcd_probe_hba+0xce0/0x10d0
> > > [    0.456349]  ufshcd_async_scan+0x34/0x310
> > > [    0.456609]  async_run_entry_fn+0x34/0x130
> > > [    0.456873]  process_one_work+0x1e4/0x434
> > > [    0.457136]  worker_thread+0x174/0x4dc
> > > [    0.457379]  kthread+0xdc/0xe0
> > > [    0.457580]  ret_from_fork+0x10/0x20
> > >
> > > which leads me to this call in ufshpb_get_dev_info():
> > >
> > > 2622         ret = ufshcd_query_attr_retry(hba,
> > > UPIU_QUERY_OPCODE_READ_ATTR,
> > > 2623                 QUERY_ATTR_IDN_MAX_HPB_SINGLE_CMD, 0, 0,
> > > &max_single_cmd);
> > >
> > > And from here on, I miss the UFS experience to debug further. But I
> > > will happily provide more information if people give me pointers.
> > Ah ok.
> > That's cool - HPB is enable on your platform.

Thank you for the information. I disabled CONFIG_SCSI_UFS_HPB on my environment.
After I enabled the config, it also output the messages.

> > For some reason JEDEC didn't merge the HPB amendment into UFS4.0 - and I
> > forgot all about that attribute.
> And the source of this error is that your device does not support HPB2.0,
> Which is fine, because HPB2.0 support was removed a while ago.

According to Device Descriptor information of my environment, the UFS device
itself seems to support HPB2.0.
-----
Device Descriptor [Byte offset 0x1f]: bUFSFeaturesSupport = 0xff  <--- so device supports HPB
...
Device Descriptor [Byte offset 0x40]: wHPBVersion = 0x200
Device Descriptor [Byte offset 0x42]: bHPBControl = 0x1
-----

Anyway, the UFS driver seems to work correctly, IIUC.

Best regards,
Yoshihiro Shimoda

> Thanks,
> Avri
> 
> >
> > Thanks,
> > Avri
> >
> > >
> > > All the best,
> > >
> > >    Wolfram
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/r8a779f0-spider-cpu.dtsi b/arch/arm64/boot/dts/renesas/r8a779f0-spider-cpu.dtsi
index 41aa8591b3b1..999c823719bc 100644
--- a/arch/arm64/boot/dts/renesas/r8a779f0-spider-cpu.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779f0-spider-cpu.dtsi
@@ -82,3 +82,11 @@  &scif3 {
 &scif_clk {
 	clock-frequency = <24000000>;
 };
+
+&ufs {
+	status = "okay";
+};
+
+&ufs30_clk {
+	clock-frequency = <38400000>;
+};