From: Alim Akhtar <alim.akhtar@gmail.com> To: Rob Herring <robh@kernel.org> Cc: Alim Akhtar <alim.akhtar@samsung.com>, devicetree@vger.kernel.org, linux-scsi@vger.kernel.org, Krzysztof Kozlowski <krzk@kernel.org>, Avri Altman <avri.altman@wdc.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Kiwoong Kim <kwmad.kim@samsung.com>, Stanley Chu <stanley.chu@mediatek.com>, Can Guo <cang@codeaurora.org>, linux-samsung-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v4 3/5] Documentation: devicetree: ufs: Add DT bindings for exynos UFS host controller Date: Sun, 12 Apr 2020 11:11:19 +0530 [thread overview] Message-ID: <CAGOxZ52nCF92adSk3a64qjZfMxXd3qHRayWZzuTRwNjcd2cC1w@mail.gmail.com> (raw) In-Reply-To: <20200405020208.GA22609@bogus> Hi Rob On Sun, Apr 5, 2020 at 7:33 AM Rob Herring <robh@kernel.org> wrote: > > On Fri, Mar 27, 2020 at 10:36:36PM +0530, Alim Akhtar wrote: > > This adds Exynos Universal Flash Storage (UFS) Host Controller DT bindings. > > Why the inconsistent subject. 'dt-bindings: ...' please. > Sure will update > > > > Signed-off-by: Seungwon Jeon <essuuj@gmail.com> > > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > > --- > > .../devicetree/bindings/ufs/ufs-exynos.txt | 104 ++++++++++++++++++ > > 1 file changed, 104 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/ufs/ufs-exynos.txt > > Use DT schema format. Not sure why you'd do that for one and not the > other... > Yah, this is my 1st attempt to write binding in DT schema format, few things were not clear, now with your review, things got clear. I will keep the common UFS binding as it is and change exynos UFS binding in schema format. Will post the changes soon. > > > > diff --git a/Documentation/devicetree/bindings/ufs/ufs-exynos.txt b/Documentation/devicetree/bindings/ufs/ufs-exynos.txt > > new file mode 100644 > > index 000000000000..08e2d1497b1b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/ufs/ufs-exynos.txt > > @@ -0,0 +1,104 @@ > > +* Exynos Universal Flash Storage (UFS) Host Controller > > + > > +UFSHC nodes are defined to describe on-chip UFS host controllers. > > +Each UFS controller instance should have its own node. > > + > > +Required properties: > > +- compatible : compatible name, contains "samsung,exynos7-ufs" > > +- interrupts : <interrupt mapping for UFS host controller IRQ> > > +- reg : Should contain HCI, vendor specific, UNIPRO and > > + UFS protector address space > > +- reg-names : "hci", "vs_hci", "unipro", "ufsp"; > > + > > +Optional properties: > > +- vdd-hba-supply : phandle to UFS host controller supply regulator node > > +- vcc-supply : phandle to VCC supply regulator node > > +- vccq-supply : phandle to VCCQ supply regulator node > > +- vccq2-supply : phandle to VCCQ2 supply regulator node > > +- vcc-supply-1p8 : For embedded UFS devices, valid VCC range is 1.7-1.95V > > + or 2.7-3.6V. This boolean property when set, specifies > > + to use low voltage range of 1.7-1.95V. Note for external > > + UFS cards this property is invalid and valid VCC range is > > + always 2.7-3.6V. > > The supply for vcc-supply should be restricted to the valid range and > this is not needed. > For now, I will leave these common binding as it is. > > +- vcc-max-microamp : specifies max. load that can be drawn from vcc supply > > +- vccq-max-microamp : specifies max. load that can be drawn from vccq supply > > +- vccq2-max-microamp : specifies max. load that can be drawn from vccq2 supply > > How is this information useful? > > > +- <name>-fixed-regulator : boolean property specifying that <name>-supply is a fixed regulator > > No need for this. Look up the phandle and check supply's node if you > want to know this. > ok > > + > > +- clocks : List of phandle and clock specifier pairs > > +- clock-names : List of clock input name strings sorted in the same > > + order as the clocks property. > > + "core", "sclk_unipro_main", "ref" and ref_parent > > + > > +- freq-table-hz : Array of <min max> operating frequencies stored in the same > > + order as the clocks property. If this property is not > > + defined or a value in the array is "0" then it is assumed > > + that the frequency is set by the parent clock or a > > + fixed rate clock source. > > +- pclk-freq-avail-range : specifies available frequency range(min/max) for APB clock > > +- ufs,pwr-attr-mode : specifies mode value for power mode change, possible values are > > + "FAST", "SLOW", "FAST_auto" and "SLOW_auto" > > Anything before the ',' is considered a vendor prefix and 'ufs' is not a > vendor. > > If these are standard UFS properties, then they should be documented in > a common UFS binding. On the flip side, none of the other UFS bindings > have needed these properties, so why do you? > Yah understood, these are not UFS common properties, I will change the driver instead to handle them. This will also simply exynos UFS binding. > > +- ufs,pwr-attr-lane : specifies lane count value for power mode change > > + allowed values are 1 or 2 > > +- ufs,pwr-attr-gear : specifies gear count value for power mode change > > + allowed values are 1 or 2 > > +- ufs,pwr-attr-hs-series : specifies HS rate series for power mode change > > + can be one of "HS_rate_b" or "HS_rate_a" > > +- ufs,pwr-local-l2-timer : specifies array of local UNIPRO L2 timer values > > + 3 timers supported > > + <FC0ProtectionTimeOutVal,TC0ReplayTimeOutVal, AFC0ReqTimeOutVal> > > +- ufs,pwr-remote-l2-timer : specifies array of remote UNIPRO L2 timer values > > + 3 timers supported > > + <FC0ProtectionTimeOutVal,TC0ReplayTimeOutVal, AFC0ReqTimeOutVal> > > +- ufs-rx-adv-fine-gran-sup_en : specifies support of fine granularity of MPHY, > > + this is a boolean property. > > +- ufs-rx-adv-fine-gran-step : specifies granularity steps of MPHY, > > + allowed step size is 0 to 3 > > +- ufs-rx-adv-min-activate-time-cap : specifies rx advanced minimum activate time of MPHY > > + range is 1 to 9 > > +- ufs-pa-granularity : specifies Granularity for PA_TActivate and PA_Hibern8Time > > +- ufs-pa-tacctivate : specifies time to wake-up remote M-RX > > +- ufs-pa-hibern8time : specifies minimum time to wait in HIBERN8 state > > + > > +Note: If above properties are not defined it can be assumed that the supply > > +regulators or clocks are always on. > > + > > +Example: > > + ufshc@0x15570000 { > > + compatible = "samsung,exynos7-ufs"; > > + reg = <0x15570000 0x100>, > > + <0x15570100 0x100>, > > + <0x15571000 0x200>, > > + <0x15572000 0x300>; > > + reg-names = "hci", "vs_hci", "unipro", "ufsp"; > > + interrupts = <0 200 0>; > > + > > + vdd-hba-supply = <&xxx_reg0>; > > + vdd-hba-fixed-regulator; > > + vcc-supply = <&xxx_reg1>; > > + vcc-supply-1p8; > > + vccq-supply = <&xxx_reg2>; > > + vccq2-supply = <&xxx_reg3>; > > + vcc-max-microamp = 500000; > > + vccq-max-microamp = 200000; > > + vccq2-max-microamp = 200000; > > + > > + clocks = <&core 0>, <&ref 0>, <&iface 0>; > > + clock-names = "core", "sclk_unipro_main", "ref", "ref_parent"; > > + freq-table-hz = <100000000 200000000>, <0 0>, <0 0>, <0 0>; > > + > > + pclk-freq-avail-range = <70000000 133000000>; > > + > > + ufs,pwr-attr-mode = "FAST"; > > + ufs,pwr-attr-lane = <2>; > > + ufs,pwr-attr-gear = <2>; > > + ufs,pwr-attr-hs-series = "HS_rate_b"; > > + ufs,pwr-local-l2-timer = <8000 28000 20000>; > > + ufs,pwr-remote-l2-timer = <12000 32000 16000>; > > + ufs-rx-adv-fine-gran-sup_en = <1>; > > + ufs-rx-adv-fine-gran-step = <3>; > > + ufs-rx-adv-min-activate-time-cap = <9>; > > + ufs-pa-granularity = <6>; > > + ufs-pa-tacctivate = <6>; > > + ufs-pa-hibern8time = <20>; > > + }; > > -- > > 2.17.1 > > -- Regards, Alim
WARNING: multiple messages have this Message-ID (diff)
From: Alim Akhtar <alim.akhtar@gmail.com> To: Rob Herring <robh@kernel.org> Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-scsi@vger.kernel.org, "Martin K. Petersen" <martin.petersen@oracle.com>, open list <linux-kernel@vger.kernel.org>, Krzysztof Kozlowski <krzk@kernel.org>, Kiwoong Kim <kwmad.kim@samsung.com>, Avri Altman <avri.altman@wdc.com>, Can Guo <cang@codeaurora.org>, Alim Akhtar <alim.akhtar@samsung.com>, Stanley Chu <stanley.chu@mediatek.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 3/5] Documentation: devicetree: ufs: Add DT bindings for exynos UFS host controller Date: Sun, 12 Apr 2020 11:11:19 +0530 [thread overview] Message-ID: <CAGOxZ52nCF92adSk3a64qjZfMxXd3qHRayWZzuTRwNjcd2cC1w@mail.gmail.com> (raw) In-Reply-To: <20200405020208.GA22609@bogus> Hi Rob On Sun, Apr 5, 2020 at 7:33 AM Rob Herring <robh@kernel.org> wrote: > > On Fri, Mar 27, 2020 at 10:36:36PM +0530, Alim Akhtar wrote: > > This adds Exynos Universal Flash Storage (UFS) Host Controller DT bindings. > > Why the inconsistent subject. 'dt-bindings: ...' please. > Sure will update > > > > Signed-off-by: Seungwon Jeon <essuuj@gmail.com> > > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > > --- > > .../devicetree/bindings/ufs/ufs-exynos.txt | 104 ++++++++++++++++++ > > 1 file changed, 104 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/ufs/ufs-exynos.txt > > Use DT schema format. Not sure why you'd do that for one and not the > other... > Yah, this is my 1st attempt to write binding in DT schema format, few things were not clear, now with your review, things got clear. I will keep the common UFS binding as it is and change exynos UFS binding in schema format. Will post the changes soon. > > > > diff --git a/Documentation/devicetree/bindings/ufs/ufs-exynos.txt b/Documentation/devicetree/bindings/ufs/ufs-exynos.txt > > new file mode 100644 > > index 000000000000..08e2d1497b1b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/ufs/ufs-exynos.txt > > @@ -0,0 +1,104 @@ > > +* Exynos Universal Flash Storage (UFS) Host Controller > > + > > +UFSHC nodes are defined to describe on-chip UFS host controllers. > > +Each UFS controller instance should have its own node. > > + > > +Required properties: > > +- compatible : compatible name, contains "samsung,exynos7-ufs" > > +- interrupts : <interrupt mapping for UFS host controller IRQ> > > +- reg : Should contain HCI, vendor specific, UNIPRO and > > + UFS protector address space > > +- reg-names : "hci", "vs_hci", "unipro", "ufsp"; > > + > > +Optional properties: > > +- vdd-hba-supply : phandle to UFS host controller supply regulator node > > +- vcc-supply : phandle to VCC supply regulator node > > +- vccq-supply : phandle to VCCQ supply regulator node > > +- vccq2-supply : phandle to VCCQ2 supply regulator node > > +- vcc-supply-1p8 : For embedded UFS devices, valid VCC range is 1.7-1.95V > > + or 2.7-3.6V. This boolean property when set, specifies > > + to use low voltage range of 1.7-1.95V. Note for external > > + UFS cards this property is invalid and valid VCC range is > > + always 2.7-3.6V. > > The supply for vcc-supply should be restricted to the valid range and > this is not needed. > For now, I will leave these common binding as it is. > > +- vcc-max-microamp : specifies max. load that can be drawn from vcc supply > > +- vccq-max-microamp : specifies max. load that can be drawn from vccq supply > > +- vccq2-max-microamp : specifies max. load that can be drawn from vccq2 supply > > How is this information useful? > > > +- <name>-fixed-regulator : boolean property specifying that <name>-supply is a fixed regulator > > No need for this. Look up the phandle and check supply's node if you > want to know this. > ok > > + > > +- clocks : List of phandle and clock specifier pairs > > +- clock-names : List of clock input name strings sorted in the same > > + order as the clocks property. > > + "core", "sclk_unipro_main", "ref" and ref_parent > > + > > +- freq-table-hz : Array of <min max> operating frequencies stored in the same > > + order as the clocks property. If this property is not > > + defined or a value in the array is "0" then it is assumed > > + that the frequency is set by the parent clock or a > > + fixed rate clock source. > > +- pclk-freq-avail-range : specifies available frequency range(min/max) for APB clock > > +- ufs,pwr-attr-mode : specifies mode value for power mode change, possible values are > > + "FAST", "SLOW", "FAST_auto" and "SLOW_auto" > > Anything before the ',' is considered a vendor prefix and 'ufs' is not a > vendor. > > If these are standard UFS properties, then they should be documented in > a common UFS binding. On the flip side, none of the other UFS bindings > have needed these properties, so why do you? > Yah understood, these are not UFS common properties, I will change the driver instead to handle them. This will also simply exynos UFS binding. > > +- ufs,pwr-attr-lane : specifies lane count value for power mode change > > + allowed values are 1 or 2 > > +- ufs,pwr-attr-gear : specifies gear count value for power mode change > > + allowed values are 1 or 2 > > +- ufs,pwr-attr-hs-series : specifies HS rate series for power mode change > > + can be one of "HS_rate_b" or "HS_rate_a" > > +- ufs,pwr-local-l2-timer : specifies array of local UNIPRO L2 timer values > > + 3 timers supported > > + <FC0ProtectionTimeOutVal,TC0ReplayTimeOutVal, AFC0ReqTimeOutVal> > > +- ufs,pwr-remote-l2-timer : specifies array of remote UNIPRO L2 timer values > > + 3 timers supported > > + <FC0ProtectionTimeOutVal,TC0ReplayTimeOutVal, AFC0ReqTimeOutVal> > > +- ufs-rx-adv-fine-gran-sup_en : specifies support of fine granularity of MPHY, > > + this is a boolean property. > > +- ufs-rx-adv-fine-gran-step : specifies granularity steps of MPHY, > > + allowed step size is 0 to 3 > > +- ufs-rx-adv-min-activate-time-cap : specifies rx advanced minimum activate time of MPHY > > + range is 1 to 9 > > +- ufs-pa-granularity : specifies Granularity for PA_TActivate and PA_Hibern8Time > > +- ufs-pa-tacctivate : specifies time to wake-up remote M-RX > > +- ufs-pa-hibern8time : specifies minimum time to wait in HIBERN8 state > > + > > +Note: If above properties are not defined it can be assumed that the supply > > +regulators or clocks are always on. > > + > > +Example: > > + ufshc@0x15570000 { > > + compatible = "samsung,exynos7-ufs"; > > + reg = <0x15570000 0x100>, > > + <0x15570100 0x100>, > > + <0x15571000 0x200>, > > + <0x15572000 0x300>; > > + reg-names = "hci", "vs_hci", "unipro", "ufsp"; > > + interrupts = <0 200 0>; > > + > > + vdd-hba-supply = <&xxx_reg0>; > > + vdd-hba-fixed-regulator; > > + vcc-supply = <&xxx_reg1>; > > + vcc-supply-1p8; > > + vccq-supply = <&xxx_reg2>; > > + vccq2-supply = <&xxx_reg3>; > > + vcc-max-microamp = 500000; > > + vccq-max-microamp = 200000; > > + vccq2-max-microamp = 200000; > > + > > + clocks = <&core 0>, <&ref 0>, <&iface 0>; > > + clock-names = "core", "sclk_unipro_main", "ref", "ref_parent"; > > + freq-table-hz = <100000000 200000000>, <0 0>, <0 0>, <0 0>; > > + > > + pclk-freq-avail-range = <70000000 133000000>; > > + > > + ufs,pwr-attr-mode = "FAST"; > > + ufs,pwr-attr-lane = <2>; > > + ufs,pwr-attr-gear = <2>; > > + ufs,pwr-attr-hs-series = "HS_rate_b"; > > + ufs,pwr-local-l2-timer = <8000 28000 20000>; > > + ufs,pwr-remote-l2-timer = <12000 32000 16000>; > > + ufs-rx-adv-fine-gran-sup_en = <1>; > > + ufs-rx-adv-fine-gran-step = <3>; > > + ufs-rx-adv-min-activate-time-cap = <9>; > > + ufs-pa-granularity = <6>; > > + ufs-pa-tacctivate = <6>; > > + ufs-pa-hibern8time = <20>; > > + }; > > -- > > 2.17.1 > > -- Regards, Alim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-04-12 5:42 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20200327171411epcas5p17f4457f9fd61800257607059b9506fb2@epcas5p1.samsung.com> 2020-03-27 17:06 ` [PATCH v4 0/5] exynos-ufs: Add support for UFS HCI Alim Akhtar 2020-03-27 17:06 ` Alim Akhtar [not found] ` <CGME20200327171414epcas5p1460e932c0bc98f31ebdd115218b4fd49@epcas5p1.samsung.com> 2020-03-27 17:06 ` [PATCH v4 1/5] dt-bindings: phy: Document Samsung UFS PHY bindings Alim Akhtar 2020-03-27 17:06 ` Alim Akhtar 2020-04-05 1:54 ` Rob Herring 2020-04-05 1:54 ` Rob Herring 2020-04-07 0:28 ` Alim Akhtar 2020-04-07 0:28 ` Alim Akhtar [not found] ` <CGME20200327171416epcas5p43133a28159ef24b145fcc8f3df102dde@epcas5p4.samsung.com> 2020-03-27 17:06 ` [PATCH v4 2/5] phy: samsung-ufs: add UFS PHY driver for samsung SoC Alim Akhtar 2020-03-27 17:06 ` Alim Akhtar [not found] ` <CGME20200327171418epcas5p4b85bea273e17c05a7edca58f528c435a@epcas5p4.samsung.com> 2020-03-27 17:06 ` [PATCH v4 3/5] Documentation: devicetree: ufs: Add DT bindings for exynos UFS host controller Alim Akhtar 2020-03-27 17:06 ` Alim Akhtar 2020-04-05 2:02 ` Rob Herring 2020-04-05 2:02 ` Rob Herring 2020-04-12 5:41 ` Alim Akhtar [this message] 2020-04-12 5:41 ` Alim Akhtar [not found] ` <CGME20200327171420epcas5p490e1e6d090a540eaf050e0728a39ba25@epcas5p4.samsung.com> 2020-03-27 17:06 ` [PATCH v4 4/5] scsi: ufs-exynos: add UFS host support for Exynos SoCs Alim Akhtar 2020-03-27 17:06 ` Alim Akhtar 2020-03-28 11:28 ` Avri Altman 2020-03-28 11:28 ` Avri Altman 2020-04-02 15:08 ` Alim Akhtar 2020-04-02 15:08 ` Alim Akhtar [not found] ` <CGME20200327171423epcas5p485d227f19e45999ad9b42b21d2864e4a@epcas5p4.samsung.com> 2020-03-27 17:06 ` [PATCH v4 5/5] arm64: dts: Add node for ufs exynos7 Alim Akhtar 2020-03-27 17:06 ` Alim Akhtar 2020-03-28 13:30 ` Paweł Chmiel 2020-03-28 13:30 ` Paweł Chmiel 2020-03-28 15:35 ` Alim Akhtar 2020-03-28 15:35 ` Alim Akhtar 2020-03-28 17:46 ` Paweł Chmiel 2020-03-28 17:46 ` Paweł Chmiel 2020-03-29 15:35 ` Alim Akhtar 2020-03-29 15:35 ` Alim Akhtar 2020-04-03 16:52 ` Paweł Chmiel 2020-04-03 16:52 ` Paweł Chmiel 2020-04-04 18:15 ` Alim Akhtar 2020-04-04 18:15 ` Alim Akhtar 2020-04-04 19:33 ` Paweł Chmiel 2020-04-04 19:33 ` Paweł Chmiel 2020-04-04 20:25 ` Paweł Chmiel 2020-04-04 20:25 ` Paweł Chmiel 2020-04-05 1:48 ` Alim Akhtar 2020-04-05 1:48 ` Alim Akhtar 2020-04-05 8:11 ` Paweł Chmiel 2020-04-05 8:11 ` Paweł Chmiel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAGOxZ52nCF92adSk3a64qjZfMxXd3qHRayWZzuTRwNjcd2cC1w@mail.gmail.com \ --to=alim.akhtar@gmail.com \ --cc=alim.akhtar@samsung.com \ --cc=avri.altman@wdc.com \ --cc=cang@codeaurora.org \ --cc=devicetree@vger.kernel.org \ --cc=krzk@kernel.org \ --cc=kwmad.kim@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=robh@kernel.org \ --cc=stanley.chu@mediatek.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.