| Message ID | 20220603110524.1997825-8-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | treewide: ufs: Add support for Renesas R-Car UFS controller | expand |
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
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!
> 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!
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!
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
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
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
> 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
> > 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
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
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 --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>; +};