答复: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
Hi, Arnd
Sorry to bother you, some questions about this patch will trouble you to give
some advice:
+ ufs: ufs@ff3b {
+ compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
+ /* 0: HCI standard */
+ /* 1: UFS SYS CTRL */
+ reg = <0x0 0xff3b 0x0 0x1000>,
+ <0x0 0xff3b1000 0x0 0x1000>;
+ interrupt-parent = <>;
+ interrupts = ;
+ clocks = <_ctrl HI3660_CLK_GATE_UFSIO_REF>,
+ <_ctrl HI3660_CLK_GATE_UFSPHY_CFG>;
+ clock-names = "clk_ref", "clk_phy";
+ freq-table-hz = <0 0>, <0 0>;
+ /* offset: 0x84; bit: 12 */
+ /* offset: 0x84; bit: 7 */
+ resets = <_rst 0x84 12>,
+ <_rst 0x84 7>;
+ reset-names = "rst", "assert";
+ };
1. our UFS host soc implementation can be divided into two parts: UFS
controller and related peripheral circuit, that "HCI standard"<-> UFS
controller, "UFS SYS CTRL"<-> related peripheral circuit, and PHY is part of
the peripheral circuit. So the "UFS SYS CTRL" area does not correspond
completely to what Qualcomm have described as their PHY implementation. In
fact, we do not have an independent register space to control the PHY.
2. From our soc chip colleague, "rst", "assert" is not generic and related with
our soc implementation. In fact，it is not just a rst and assert of the UFS
controller, but for the entire UFS IP ,so I don't think it's very helpful for
others.
I think the above places will be reserved, do you have any better advices.
Thank you very much.
-邮件原件-
发件人: liwei (CM)
发送时间: 2017年10月31日 20:35
收件人: 'Arnd Bergmann'
抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; Vinayak
Holikatti; James E.J. Bottomley; Martin K. Petersen; Kevin Hilman; Gregory
CLEMENT; Thomas Petazzoni; Masahiro Yamada; Riku Voipio; Thierry Reding;
Krzysztof Kozlowski; Eric Anholt; DTML; Linux Kernel Mailing List; Linux ARM;
linux-scsi; Guodong Xu; Fengbaopeng (kevin, Kirin Solution Dept); lihuan (Z);
wangyupeng (A); zangleigang
主题: 答复: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
Hi, Arnd
Thank you for your reply.
-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2017年10月30日 23:22
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; Vinayak
Holikatti; James E.J. Bottomley; Martin K. Petersen; Kevin Hilman; Gregory
CLEMENT; Thomas Petazzoni; Masahiro Yamada; Riku Voipio; Thierry Reding;
Krzysztof Kozlowski; Eric Anholt; DTML; Linux Kernel Mailing List; Linux ARM;
linux-scsi; Guodong Xu; Fengbaopeng (kevin, Kirin Solution Dept); lihuan (Z);
wangyupeng (A)
主题: Re: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
On Tue, Oct 24, 2017 at 11:06 AM, liwei (CM) <liwei...@huawei.com> wrote:
> what's your opinion about my explanation and revision method?
> I am looking forward to your reply, thanks!
Sorry for the delay, I was travelling last week.
> 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd
> Bergmann On Fri, Oct 20, 2017 at 10:52 AM, Li Wei <liwei...@huawei.com> wrote:
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> @@ -0,0 +1,46 @@
>> +* Hisilicon Universal Flash Storage (UFS) Host Controller
>> +
>> +UFS nodes are defined to describe on-chip UFS hardware macro.
>> +Each UFS Host Controller should have its own node.
>> +
>> +Required properties:
>> +- compatible: compatible list, contains one of the following -
>> + "hisilicon,hi3660-ufs" for hisi ufs host controller
>> +present on Hi3660 chipset.
One more thing I just noticed: you don't describe the device as compatible with
the ufshcd spec as defined in the generic binding. Shouldn't we have both
compatible strings listed here?
Ok, I will fix it in patch v6;
> In particular, I wonder if what you describe as the "UFS SYS CTRL"
> area corresponds to what Qualcomm have described as their PHY implementation.
> It certainly seems to driver some of the properties that would normally be
> associated with a PHY.
>
> Liwei:Yes, a part of "UFS SYS CTRL" is associated with a PHY, but from our
> chip colleague that we assure "UFS SYS CTRL" is associated with
> clk/reset/power on/power off and so on.
> In fact, in addition to the controller itself, the controller related
> periphery are all in this area. So it's not appropriate to put this into a
> separate phy node.
答复: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
Hi, Arnd
Sorry to bother you, some questions about this patch will trouble you to give
some advice:
+ ufs: ufs@ff3b {
+ compatible = "hisilicon,hi3660-ufs", "jedec,ufs-1.1";
+ /* 0: HCI standard */
+ /* 1: UFS SYS CTRL */
+ reg = <0x0 0xff3b 0x0 0x1000>,
+ <0x0 0xff3b1000 0x0 0x1000>;
+ interrupt-parent = <>;
+ interrupts = ;
+ clocks = <_ctrl HI3660_CLK_GATE_UFSIO_REF>,
+ <_ctrl HI3660_CLK_GATE_UFSPHY_CFG>;
+ clock-names = "clk_ref", "clk_phy";
+ freq-table-hz = <0 0>, <0 0>;
+ /* offset: 0x84; bit: 12 */
+ /* offset: 0x84; bit: 7 */
+ resets = <_rst 0x84 12>,
+ <_rst 0x84 7>;
+ reset-names = "rst", "assert";
+ };
1. our UFS host soc implementation can be divided into two parts: UFS
controller and related peripheral circuit, that "HCI standard"<-> UFS
controller, "UFS SYS CTRL"<-> related peripheral circuit, and PHY is part of
the peripheral circuit. So the "UFS SYS CTRL" area does not correspond
completely to what Qualcomm have described as their PHY implementation. In
fact, we do not have an independent register space to control the PHY.
2. From our soc chip colleague, "rst", "assert" is not generic and related with
our soc implementation. In fact，it is not just a rst and assert of the UFS
controller, but for the entire UFS IP ,so I don't think it's very helpful for
others.
I think the above places will be reserved, do you have any better advices.
Thank you very much.
-邮件原件-
发件人: liwei (CM)
发送时间: 2017年10月31日 20:35
收件人: 'Arnd Bergmann'
抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; Vinayak
Holikatti; James E.J. Bottomley; Martin K. Petersen; Kevin Hilman; Gregory
CLEMENT; Thomas Petazzoni; Masahiro Yamada; Riku Voipio; Thierry Reding;
Krzysztof Kozlowski; Eric Anholt; DTML; Linux Kernel Mailing List; Linux ARM;
linux-scsi; Guodong Xu; Fengbaopeng (kevin, Kirin Solution Dept); lihuan (Z);
wangyupeng (A); zangleigang
主题: 答复: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
Hi, Arnd
Thank you for your reply.
-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2017年10月30日 23:22
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; Vinayak
Holikatti; James E.J. Bottomley; Martin K. Petersen; Kevin Hilman; Gregory
CLEMENT; Thomas Petazzoni; Masahiro Yamada; Riku Voipio; Thierry Reding;
Krzysztof Kozlowski; Eric Anholt; DTML; Linux Kernel Mailing List; Linux ARM;
linux-scsi; Guodong Xu; Fengbaopeng (kevin, Kirin Solution Dept); lihuan (Z);
wangyupeng (A)
主题: Re: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
On Tue, Oct 24, 2017 at 11:06 AM, liwei (CM) wrote:
> what's your opinion about my explanation and revision method?
> I am looking forward to your reply, thanks!
Sorry for the delay, I was travelling last week.
> 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd
> Bergmann On Fri, Oct 20, 2017 at 10:52 AM, Li Wei wrote:
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt
>> @@ -0,0 +1,46 @@
>> +* Hisilicon Universal Flash Storage (UFS) Host Controller
>> +
>> +UFS nodes are defined to describe on-chip UFS hardware macro.
>> +Each UFS Host Controller should have its own node.
>> +
>> +Required properties:
>> +- compatible: compatible list, contains one of the following -
>> + "hisilicon,hi3660-ufs" for hisi ufs host controller
>> +present on Hi3660 chipset.
One more thing I just noticed: you don't describe the device as compatible with
the ufshcd spec as defined in the generic binding. Shouldn't we have both
compatible strings listed here?
Ok, I will fix it in patch v6;
> In particular, I wonder if what you describe as the "UFS SYS CTRL"
> area corresponds to what Qualcomm have described as their PHY implementation.
> It certainly seems to driver some of the properties that would normally be
> associated with a PHY.
>
> Liwei:Yes, a part of "UFS SYS CTRL" is associated with a PHY, but from our
> chip colleague that we assure "UFS SYS CTRL" is associated with
> clk/reset/power on/power off and so on.
> In fact, in addition to the controller itself, the controller related
> periphery are all in this area. So it's not appropriate to put this into a
> separate phy node.
I'm not sure I understand here. Do you mean the reset
答复: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
Hi, Arnd Thank you for your reply. -邮件原件- 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann 发送时间: 2017年10月30日 23:22 收件人: liwei (CM) 抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; Vinayak Holikatti; James E.J. Bottomley; Martin K. Petersen; Kevin Hilman; Gregory CLEMENT; Thomas Petazzoni; Masahiro Yamada; Riku Voipio; Thierry Reding; Krzysztof Kozlowski; Eric Anholt; DTML; Linux Kernel Mailing List; Linux ARM; linux-scsi; Guodong Xu; Fengbaopeng (kevin, Kirin Solution Dept); lihuan (Z); wangyupeng (A) 主题: Re: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs On Tue, Oct 24, 2017 at 11:06 AM, liwei (CM) <liwei...@huawei.com> wrote: > what's your opinion about my explanation and revision method? > I am looking forward to your reply, thanks! Sorry for the delay, I was travelling last week. > 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd > Bergmann On Fri, Oct 20, 2017 at 10:52 AM, Li Wei <liwei...@huawei.com> wrote: >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt >> @@ -0,0 +1,46 @@ >> +* Hisilicon Universal Flash Storage (UFS) Host Controller >> + >> +UFS nodes are defined to describe on-chip UFS hardware macro. >> +Each UFS Host Controller should have its own node. >> + >> +Required properties: >> +- compatible: compatible list, contains one of the following - >> + "hisilicon,hi3660-ufs" for hisi ufs host controller >> +present on Hi3660 chipset. One more thing I just noticed: you don't describe the device as compatible with the ufshcd spec as defined in the generic binding. Shouldn't we have both compatible strings listed here? Ok, I will fix it in patch v6; > In particular, I wonder if what you describe as the "UFS SYS CTRL" > area corresponds to what Qualcomm have described as their PHY implementation. > It certainly seems to driver some of the properties that would normally be > associated with a PHY. > > Liwei:Yes, a part of "UFS SYS CTRL" is associated with a PHY, but from our > chip colleague that we assure "UFS SYS CTRL" is associated with > clk/reset/power on/power off and so on. > In fact, in addition to the controller itself, the controller related > periphery are all in this area. So it's not appropriate to put this into a > separate phy node. I'm not sure I understand here. Do you mean the reset handle is for resetting the PHY rather than the UFS HCD? Maybe my description is not clear enough, our UFS host soc implementation can be divided into two parts: UFS controller and related peripheral circuit, that "HCI standard"<-> UFS controller, "UFS SYS CTRL"<-> related peripheral circuit, and PHY is part of the peripheral circuit. So the "UFS SYS CTRL" area does not correspond completely to what Qualcomm have described as their PHY implementation. The root reason is that our UFS host had not divided into UFS controller and PHY； > > For the "clock-names" property, you specify "clk_ref", which I assume is > > the same as what Qualcomm call "ref_clk". I'd suggest you use the existing > > name and add that as the default name in the ufshcd-pltfrm.txt binding > > document. > > Liwei:" ref_clk " is already in the ufshcd-pltfrm.txt binding > document, and parse in ufshcd.c, so we will replace "clk_ref" with > "ref_clk". I will fix it in patch v6; ok > > The "clk_phy" property appears to be related to the PHY, so it might be > > better to have a separate phy node with either just the clk, or with the > > clk plus the "UFS SYS CTRL" register area, whichever matches your hardware > > better, and then use teh "phys/phy-names" property to refer to that. > > Liwei: OK, I will add a separate phy node and fix it in patch v6; Thanks. >> The reset handling you describe here (both resets and reset-gpios) appears >> to be completely generic, so I'd suggest adding those to ufshcd-pltfrm.txt >> instead of your own binding, to ensure that future drivers use the same >> identifiers. > > Liwei: From our soc chip colleague, reset include "rst", "assert" is > not generic and related with our soc implementation, you can see > ufs_hisi_soc_init() in drivers/scsi/ufs/ufs-hisi.c, the position of > rst and assert is very special, it's hard to put it in a generic > process; It seems odd that the ability to reset a device is specific to your implementation. What I meant is that other implementations may also require a reset, so describing the way you refer to this into t
答复: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
Hi, Arnd Thank you for your reply. -邮件原件- 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann 发送时间: 2017年10月30日 23:22 收件人: liwei (CM) 抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; Vinayak Holikatti; James E.J. Bottomley; Martin K. Petersen; Kevin Hilman; Gregory CLEMENT; Thomas Petazzoni; Masahiro Yamada; Riku Voipio; Thierry Reding; Krzysztof Kozlowski; Eric Anholt; DTML; Linux Kernel Mailing List; Linux ARM; linux-scsi; Guodong Xu; Fengbaopeng (kevin, Kirin Solution Dept); lihuan (Z); wangyupeng (A) 主题: Re: 答复: [PATCH v5 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs On Tue, Oct 24, 2017 at 11:06 AM, liwei (CM) wrote: > what's your opinion about my explanation and revision method? > I am looking forward to your reply, thanks! Sorry for the delay, I was travelling last week. > 发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd > Bergmann On Fri, Oct 20, 2017 at 10:52 AM, Li Wei wrote: >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/ufs/ufs-hisi.txt >> @@ -0,0 +1,46 @@ >> +* Hisilicon Universal Flash Storage (UFS) Host Controller >> + >> +UFS nodes are defined to describe on-chip UFS hardware macro. >> +Each UFS Host Controller should have its own node. >> + >> +Required properties: >> +- compatible: compatible list, contains one of the following - >> + "hisilicon,hi3660-ufs" for hisi ufs host controller >> +present on Hi3660 chipset. One more thing I just noticed: you don't describe the device as compatible with the ufshcd spec as defined in the generic binding. Shouldn't we have both compatible strings listed here? Ok, I will fix it in patch v6; > In particular, I wonder if what you describe as the "UFS SYS CTRL" > area corresponds to what Qualcomm have described as their PHY implementation. > It certainly seems to driver some of the properties that would normally be > associated with a PHY. > > Liwei:Yes, a part of "UFS SYS CTRL" is associated with a PHY, but from our > chip colleague that we assure "UFS SYS CTRL" is associated with > clk/reset/power on/power off and so on. > In fact, in addition to the controller itself, the controller related > periphery are all in this area. So it's not appropriate to put this into a > separate phy node. I'm not sure I understand here. Do you mean the reset handle is for resetting the PHY rather than the UFS HCD? Maybe my description is not clear enough, our UFS host soc implementation can be divided into two parts: UFS controller and related peripheral circuit, that "HCI standard"<-> UFS controller, "UFS SYS CTRL"<-> related peripheral circuit, and PHY is part of the peripheral circuit. So the "UFS SYS CTRL" area does not correspond completely to what Qualcomm have described as their PHY implementation. The root reason is that our UFS host had not divided into UFS controller and PHY； > > For the "clock-names" property, you specify "clk_ref", which I assume is > > the same as what Qualcomm call "ref_clk". I'd suggest you use the existing > > name and add that as the default name in the ufshcd-pltfrm.txt binding > > document. > > Liwei:" ref_clk " is already in the ufshcd-pltfrm.txt binding > document, and parse in ufshcd.c, so we will replace "clk_ref" with > "ref_clk". I will fix it in patch v6; ok > > The "clk_phy" property appears to be related to the PHY, so it might be > > better to have a separate phy node with either just the clk, or with the > > clk plus the "UFS SYS CTRL" register area, whichever matches your hardware > > better, and then use teh "phys/phy-names" property to refer to that. > > Liwei: OK, I will add a separate phy node and fix it in patch v6; Thanks. >> The reset handling you describe here (both resets and reset-gpios) appears >> to be completely generic, so I'd suggest adding those to ufshcd-pltfrm.txt >> instead of your own binding, to ensure that future drivers use the same >> identifiers. > > Liwei: From our soc chip colleague, reset include "rst", "assert" is > not generic and related with our soc implementation, you can see > ufs_hisi_soc_init() in drivers/scsi/ufs/ufs-hisi.c, the position of > rst and assert is very special, it's hard to put it in a generic > process; It seems odd that the ability to reset a device is specific to your implementation. What I meant is that other implementations may also require a reset, so describing the way you refer to this into the generic binding would be helpful, to prevent others from
