[PATCH 0/2] Add DT Support for MMCSD in Am65x-evm

[PATCH 0/2] Add DT Support for MMCSD in Am65x-evm

[Faiz Abbas]

The following patches add dt support for MMCSD on the AM65x-evm.

THe series depends on driver support[1] and pinmux support[2] posted
separately.

[1] https://patchwork.kernel.org/project/linux-mmc/list/?series=53185
[2] https://lore.kernel.org/patchwork/project/lkml/list/?series=372649

Faiz Abbas (2):
  arm64: dts: ti: k3-am654: Add Support for MMC/SD
  arm64: dts: ti: k3-am654-base-board: Add MMC\SD support

 arch/arm64/boot/dts/ti/k3-am65-main.dtsi      | 28 +++++++++++
 .../arm64/boot/dts/ti/k3-am654-base-board.dts | 46 +++++++++++++++++++
 2 files changed, 74 insertions(+)

-- 
2.19.2

[PATCH 1/2] arm64: dts: ti: k3-am654: Add Support for MMC/SD

[Faiz Abbas]

There are two MMC host controller instances present on the TI's
Am654 SOCs. Add device tree nodes for the same.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 28 ++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index 916434839603..d07212f16a81 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -129,4 +129,32 @@
 		clocks = <&k3_clks 113 1>;
 		power-domains = <&k3_pds 113>;
 	};
+
+	sdhci0: sdhci@4f80000 {
+		compatible = "ti,am654-sdhci-5.1";
+		reg = <0x0 0x4f80000 0x0 0x260>, <0x0 0x4f90000 0x0 0x134>;
+		power-domains = <&k3_pds 47>;
+		clocks = <&k3_clks 47 0>, <&k3_clks 47 1>;
+		clock-names = "clk_ahb", "clk_xin";
+		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+		sdhci-caps-mask = <0x80000007 0x0>;
+		mmc-ddr-1_8v;
+		ti,otap-del-sel = <0x2>;
+		ti,trm-icp = <0x8>;
+		status = "disabled";
+	};
+
+	sdhci1: sdhci@4fa0000 {
+		compatible = "ti,am654-sdhci-5.1";
+		reg = <0x0 0x4fa0000 0x0 0x260>, <0x0 0x4fb0000 0x0 0x134>;
+		power-domains = <&k3_pds 48>;
+		clocks = <&k3_clks 48 0>, <&k3_clks 48 1>;
+		clock-names = "clk_ahb", "clk_xin";
+		interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
+		sdhci-caps-mask = <0x80000007 0x0>;
+		mmc-ddr-1_8v;
+		ti,otap-del-sel = <0x2>;
+		ti,trm-icp = <0x8>;
+		status = "disabled";
+	};
 };
-- 
2.19.2

[PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Faiz Abbas]

On the am654x-evm, sdhci0 node is connected to an eMMC while sdhci1
is connected to an SD card slot. Add nodes and pinmuxes for the same.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 .../arm64/boot/dts/ti/k3-am654-base-board.dts | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
index bd5a0069191d..5dcd16b787eb 100644
--- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
@@ -60,6 +60,36 @@
 			AM65X_IOPAD(0x0070, PIN_INPUT, 5) /* (R25) GPMC0_CSn2.I2C2_SDA */
 		>;
 	};
+
+	main_mmc0_pins_default: main_mmc0_pins_default {
+		pinctrl-single,pins = <
+			AM65X_IOPAD(0x01a8, PIN_INPUT_PULLDOWN, 0) /* (B25) MMC0_CLK */
+			AM65X_IOPAD(0x01ac, PIN_INPUT_PULLUP, 0) /* (B27) MMC0_CMD */
+			AM65X_IOPAD(0x01a4, PIN_INPUT_PULLUP, 0) /* (A26) MMC0_DAT0 */
+			AM65X_IOPAD(0x01a0, PIN_INPUT_PULLUP, 0) /* (E25) MMC0_DAT1 */
+			AM65X_IOPAD(0x019c, PIN_INPUT_PULLUP, 0) /* (C26) MMC0_DAT2 */
+			AM65X_IOPAD(0x0198, PIN_INPUT_PULLUP, 0) /* (A25) MMC0_DAT3 */
+			AM65X_IOPAD(0x0194, PIN_INPUT_PULLUP, 0) /* (E24) MMC0_DAT4 */
+			AM65X_IOPAD(0x0190, PIN_INPUT_PULLUP, 0) /* (A24) MMC0_DAT5 */
+			AM65X_IOPAD(0x018c, PIN_INPUT_PULLUP, 0) /* (B26) MMC0_DAT6 */
+			AM65X_IOPAD(0x0188, PIN_INPUT_PULLUP, 0) /* (D25) MMC0_DAT7 */
+			AM65X_IOPAD(0x01b4, PIN_INPUT_PULLUP, 0) /* (A23) MMC0_SDCD */
+			AM65X_IOPAD(0x01b0, PIN_INPUT, 0) /* (C25) MMC0_DS */
+		>;
+	};
+
+	main_mmc1_pins_default: main_mmc1_pins_default {
+		pinctrl-single,pins = <
+			AM65X_IOPAD(0x02d4, PIN_INPUT_PULLDOWN, 0) /* (C27) MMC1_CLK */
+			AM65X_IOPAD(0x02d8, PIN_INPUT_PULLUP, 0) /* (C28) MMC1_CMD */
+			AM65X_IOPAD(0x02d0, PIN_INPUT_PULLUP, 0) /* (D28) MMC1_DAT0 */
+			AM65X_IOPAD(0x02cc, PIN_INPUT_PULLUP, 0) /* (E27) MMC1_DAT1 */
+			AM65X_IOPAD(0x02c8, PIN_INPUT_PULLUP, 0) /* (D26) MMC1_DAT2 */
+			AM65X_IOPAD(0x02c4, PIN_INPUT_PULLUP, 0) /* (D27) MMC1_DAT3 */
+			AM65X_IOPAD(0x02dc, PIN_INPUT_PULLUP, 0) /* (B24) MMC1_SDCD */
+			AM65X_IOPAD(0x02e0, PIN_INPUT, 0) /* (C24) MMC1_SDWP */
+		>;
+	};
 };
 
 &main_pmx1 {
@@ -125,3 +155,19 @@
 	pinctrl-0 = <&main_i2c2_pins_default>;
 	clock-frequency = <400000>;
 };
+
+&sdhci0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&main_mmc0_pins_default>;
+	bus-width = <8>;
+	non-removable;
+	ti,driver-strength-ohm = <50>;
+};
+
+&sdhci1 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&main_mmc1_pins_default>;
+	ti,driver-strength-ohm = <50>;
+};
-- 
2.19.2

Re: [PATCH 1/2] arm64: dts: ti: k3-am654: Add Support for MMC/SD

[Vignesh R]

On 07/12/18 2:12 PM, Faiz Abbas wrote:
> There are two MMC host controller instances present on the TI's
> Am654 SOCs. Add device tree nodes for the same.
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 28 ++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> index 916434839603..d07212f16a81 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> @@ -129,4 +129,32 @@
>  		clocks = <&k3_clks 113 1>;
>  		power-domains = <&k3_pds 113>;
>  	};
> +
> +	sdhci0: sdhci@4f80000 {
> +		compatible = "ti,am654-sdhci-5.1";
> +		reg = <0x0 0x4f80000 0x0 0x260>, <0x0 0x4f90000 0x0 0x134>;
> +		power-domains = <&k3_pds 47>;
> +		clocks = <&k3_clks 47 0>, <&k3_clks 47 1>;
> +		clock-names = "clk_ahb", "clk_xin";
> +		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
> +		sdhci-caps-mask = <0x80000007 0x0>;
> +		mmc-ddr-1_8v;
> +		ti,otap-del-sel = <0x2>;
> +		ti,trm-icp = <0x8>;
> +		status = "disabled";
> +	};

Please drop "status=disabled" from dtsi. Can be disabled as required in
the board dts.

> +
> +	sdhci1: sdhci@4fa0000 {
> +		compatible = "ti,am654-sdhci-5.1";
> +		reg = <0x0 0x4fa0000 0x0 0x260>, <0x0 0x4fb0000 0x0 0x134>;
> +		power-domains = <&k3_pds 48>;
> +		clocks = <&k3_clks 48 0>, <&k3_clks 48 1>;
> +		clock-names = "clk_ahb", "clk_xin";
> +		interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
> +		sdhci-caps-mask = <0x80000007 0x0>;
> +		mmc-ddr-1_8v;
> +		ti,otap-del-sel = <0x2>;
> +		ti,trm-icp = <0x8>;
> +		status = "disabled";
> +	};
>  };
> 

-- 
Regards
Vignesh

Re: [PATCH 1/2] arm64: dts: ti: k3-am654: Add Support for MMC/SD

[Nishanth Menon]

On 10:56-20181208, Vignesh R wrote:
> 
> 
> On 07/12/18 2:12 PM, Faiz Abbas wrote:
> > There are two MMC host controller instances present on the TI's
> > Am654 SOCs. Add device tree nodes for the same.
> > 
> > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> > ---
> >  arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 28 ++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > index 916434839603..d07212f16a81 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > @@ -129,4 +129,32 @@
> >  		clocks = <&k3_clks 113 1>;
> >  		power-domains = <&k3_pds 113>;
> >  	};
> > +
> > +	sdhci0: sdhci@4f80000 {
> > +		compatible = "ti,am654-sdhci-5.1";
> > +		reg = <0x0 0x4f80000 0x0 0x260>, <0x0 0x4f90000 0x0 0x134>;
> > +		power-domains = <&k3_pds 47>;
> > +		clocks = <&k3_clks 47 0>, <&k3_clks 47 1>;
> > +		clock-names = "clk_ahb", "clk_xin";
> > +		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
> > +		sdhci-caps-mask = <0x80000007 0x0>;
> > +		mmc-ddr-1_8v;
> > +		ti,otap-del-sel = <0x2>;
> > +		ti,trm-icp = <0x8>;
> > +		status = "disabled";
> > +	};
> 
> Please drop "status=disabled" from dtsi. Can be disabled as required in
> the board dts.


yes - the standard in k3 is to disable the nodes that are'nt needed in
board.dtsi.

This is different from "disabled by default" approach in DRA7 or OMAP4
for example.

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Nishanth Menon]

On 14:12-20181207, Faiz Abbas wrote:

> +
> +&sdhci0 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&main_mmc0_pins_default>;
> +	bus-width = <8>;
> +	non-removable;
> +	ti,driver-strength-ohm = <50>;

^^

> +};
> +
> +&sdhci1 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&main_mmc1_pins_default>;
> +	ti,driver-strength-ohm = <50>;

NAK.

$ git checkout next-20181207
$ git grep ti,driver-strength-ohm Documentation
$

Nada.. And.. I think "new phy binding" probably introduces this.
[1] https://patchwork.kernel.org/project/linux-mmc/list/?series=53185

If your patches are'nt really ready, please send them as RFC, I am not
really in a mood to track the status of every single driver subsystem.

If your binding is not in linux next at the baremin, as far as I am
concerned, this is not ready, and should be RFC.

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Sekhar Nori]

On 08/12/18 9:24 PM, Nishanth Menon wrote:
> On 14:12-20181207, Faiz Abbas wrote:
> 
>> +
>> +&sdhci0 {
>> +	status = "okay";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&main_mmc0_pins_default>;
>> +	bus-width = <8>;
>> +	non-removable;
>> +	ti,driver-strength-ohm = <50>;
> 
> ^^
> 
>> +};
>> +
>> +&sdhci1 {
>> +	status = "okay";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&main_mmc1_pins_default>;
>> +	ti,driver-strength-ohm = <50>;
> 
> NAK.
> 
> $ git checkout next-20181207
> $ git grep ti,driver-strength-ohm Documentation
> $
> 
> Nada.. And.. I think "new phy binding" probably introduces this.
> [1] https://patchwork.kernel.org/project/linux-mmc/list/?series=53185
> 
> If your patches are'nt really ready, please send them as RFC, I am not
> really in a mood to track the status of every single driver subsystem.
> 
> If your binding is not in linux next at the baremin, as far as I am
> concerned, this is not ready, and should be RFC.

No, RFC does not say "do not merge" or "this has dependencies". RFC is
used to invite a stronger review when introducing a new concept. Its
fair game to apply patches marked RFC if maintainer is okay with the
content.

Dependencies are either noted in cover-letter or below the patch
tear-line. With what you are asking, looks like patches need to be
resubmitted once dependencies are cleared, even if there is no change in
the content itself. This will be additional work.

That said, if it makes life convenient for you, you can impose such a
rule for patches you need to handle. But I think it will take some
getting used for developers who send patches to you as I don't think
this is a norm elsewhere.

Adding Tony and Arnd as well, in case I have missed some recently
accepted convention.

Thanks,
Sekhar

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Nishanth Menon]

On 13:33-20181210, Sekhar Nori wrote:
> On 08/12/18 9:24 PM, Nishanth Menon wrote:
> > On 14:12-20181207, Faiz Abbas wrote:
> > 
> >> +
> >> +&sdhci0 {
> >> +	status = "okay";
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&main_mmc0_pins_default>;
> >> +	bus-width = <8>;
> >> +	non-removable;
> >> +	ti,driver-strength-ohm = <50>;
> > 
> > ^^
> > 
> >> +};
> >> +
> >> +&sdhci1 {
> >> +	status = "okay";
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&main_mmc1_pins_default>;
> >> +	ti,driver-strength-ohm = <50>;
> > 
> > NAK.
> > 
> > $ git checkout next-20181207
> > $ git grep ti,driver-strength-ohm Documentation
> > $
> > 
> > Nada.. And.. I think "new phy binding" probably introduces this.
> > [1] https://patchwork.kernel.org/project/linux-mmc/list/?series=53185
> > 
> > If your patches are'nt really ready, please send them as RFC, I am not
> > really in a mood to track the status of every single driver subsystem.
> > 
> > If your binding is not in linux next at the baremin, as far as I am
> > concerned, this is not ready, and should be RFC.
> 
> No, RFC does not say "do not merge" or "this has dependencies". RFC is
> used to invite a stronger review when introducing a new concept. Its
> fair game to apply patches marked RFC if maintainer is okay with the
> content.

True, fair enough.. RFC is request for comments. Anyways, that is
besides the point.
> 
> Dependencies are either noted in cover-letter or below the patch
> tear-line. With what you are asking, looks like patches need to be
> resubmitted once dependencies are cleared, even if there is no change in
> the content itself. This will be additional work.

Yes please. There would be other dts changes that are probably ready and
I really wont be tracking everything happening on other drivers. If the
binding is present at least in next, it is a good indication of things
clean and ready to go.

> 
> That said, if it makes life convenient for you, you can impose such a
> rule for patches you need to handle. But I think it will take some
> getting used for developers who send patches to you as I don't think
> this is a norm elsewhere.
> 
> Adding Tony and Arnd as well, in case I have missed some recently
> accepted convention.


I have'nt looked at any conventions, The style I prefer to follow when I do
submissions: It is my job to get the bindings in, until then my actual
dts is just "request for comments". Only after the bindings are merged
do I formally submit dts - simply because I dont expect dts maintainer
to track what happened to my driver's binding and discussions there of.

Seriously, is'nt it really reasonable for dts maintainer to check every
single driver's development status in 15 different mailing lists?
Because, it sounds like what you are asking. At least I wont have time
for it..


I really am curious how Arnd / Tony actually pull this one off.. If they
have continous cron job for checking if your patch is ready... I doubt
it..

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Sekhar Nori]

Hi Nishanth,

On 10/12/18 5:36 PM, Nishanth Menon wrote:
> On 13:33-20181210, Sekhar Nori wrote:
>> On 08/12/18 9:24 PM, Nishanth Menon wrote:
>>> On 14:12-20181207, Faiz Abbas wrote:
>>>
>>>> +
>>>> +&sdhci0 {
>>>> +	status = "okay";
>>>> +	pinctrl-names = "default";
>>>> +	pinctrl-0 = <&main_mmc0_pins_default>;
>>>> +	bus-width = <8>;
>>>> +	non-removable;
>>>> +	ti,driver-strength-ohm = <50>;
>>>
>>> ^^
>>>
>>>> +};
>>>> +
>>>> +&sdhci1 {
>>>> +	status = "okay";
>>>> +	pinctrl-names = "default";
>>>> +	pinctrl-0 = <&main_mmc1_pins_default>;
>>>> +	ti,driver-strength-ohm = <50>;
>>>
>>> NAK.
>>>
>>> $ git checkout next-20181207
>>> $ git grep ti,driver-strength-ohm Documentation
>>> $
>>>
>>> Nada.. And.. I think "new phy binding" probably introduces this.
>>> [1] https://patchwork.kernel.org/project/linux-mmc/list/?series=53185
>>>
>>> If your patches are'nt really ready, please send them as RFC, I am not
>>> really in a mood to track the status of every single driver subsystem.
>>>
>>> If your binding is not in linux next at the baremin, as far as I am
>>> concerned, this is not ready, and should be RFC.
>>
>> No, RFC does not say "do not merge" or "this has dependencies". RFC is
>> used to invite a stronger review when introducing a new concept. Its
>> fair game to apply patches marked RFC if maintainer is okay with the
>> content.
> 
> True, fair enough.. RFC is request for comments. Anyways, that is
> besides the point.
>>
>> Dependencies are either noted in cover-letter or below the patch
>> tear-line. With what you are asking, looks like patches need to be
>> resubmitted once dependencies are cleared, even if there is no change in
>> the content itself. This will be additional work.
> 
> Yes please. There would be other dts changes that are probably ready and
> I really wont be tracking everything happening on other drivers. If the
> binding is present at least in next, it is a good indication of things
> clean and ready to go.

Agree that bindings should be in linux-next before device-tree files are
merged.

> 
>>
>> That said, if it makes life convenient for you, you can impose such a
>> rule for patches you need to handle. But I think it will take some
>> getting used for developers who send patches to you as I don't think
>> this is a norm elsewhere.
>>
>> Adding Tony and Arnd as well, in case I have missed some recently
>> accepted convention.
> 
> 
> I have'nt looked at any conventions, The style I prefer to follow when I do
> submissions: It is my job to get the bindings in, until then my actual
> dts is just "request for comments". Only after the bindings are merged
> do I formally submit dts - simply because I dont expect dts maintainer
> to track what happened to my driver's binding and discussions there of.

Ok.

> 
> Seriously, is'nt it really reasonable for dts maintainer to check every
> single driver's development status in 15 different mailing lists?
> Because, it sounds like what you are asking. At least I wont have time
> for it..
> 
> 
> I really am curious how Arnd / Tony actually pull this one off.. If they
> have continous cron job for checking if your patch is ready... I doubt
> it..

I think you can rely on the author to tell you when something is
actually ready to be merged (and you can tell him/her to remind you).

For the review itself, doing it by having a look at the dependencies
mentioned in the cover letter (like available for this series) should be
good enough (I feel).

I am not sure if there is a need to post an "RFC version", and then
follow it up with an actual "PATCH version" once dependencies are
cleared though.

Thanks,
Sekhar

Re: [PATCH 1/2] arm64: dts: ti: k3-am654: Add Support for MMC/SD

[Faiz Abbas]

Hi Nishanth,

On 08/12/18 9:15 PM, Nishanth Menon wrote:
> On 10:56-20181208, Vignesh R wrote:
>>
>>
>> On 07/12/18 2:12 PM, Faiz Abbas wrote:
>>> There are two MMC host controller instances present on the TI's
>>> Am654 SOCs. Add device tree nodes for the same.
>>>
>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>> ---
>>>  arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 28 ++++++++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>> index 916434839603..d07212f16a81 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>> @@ -129,4 +129,32 @@
>>>  		clocks = <&k3_clks 113 1>;
>>>  		power-domains = <&k3_pds 113>;
>>>  	};
>>> +
>>> +	sdhci0: sdhci@4f80000 {
>>> +		compatible = "ti,am654-sdhci-5.1";
>>> +		reg = <0x0 0x4f80000 0x0 0x260>, <0x0 0x4f90000 0x0 0x134>;
>>> +		power-domains = <&k3_pds 47>;
>>> +		clocks = <&k3_clks 47 0>, <&k3_clks 47 1>;
>>> +		clock-names = "clk_ahb", "clk_xin";
>>> +		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
>>> +		sdhci-caps-mask = <0x80000007 0x0>;
>>> +		mmc-ddr-1_8v;
>>> +		ti,otap-del-sel = <0x2>;
>>> +		ti,trm-icp = <0x8>;
>>> +		status = "disabled";
>>> +	};
>>
>> Please drop "status=disabled" from dtsi. Can be disabled as required in
>> the board dts.
> 
> 
> yes - the standard in k3 is to disable the nodes that are'nt needed in
> board.dtsi.
> 
> This is different from "disabled by default" approach in DRA7 or OMAP4
> for example.
> 

Ok. Will fix in v2.

Thanks,
Faiz

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Faiz Abbas]

Hi,

On 10/12/18 6:10 PM, Sekhar Nori wrote:
> Hi Nishanth,
> 
> On 10/12/18 5:36 PM, Nishanth Menon wrote:
>> On 13:33-20181210, Sekhar Nori wrote:
>>> On 08/12/18 9:24 PM, Nishanth Menon wrote:
>>>> On 14:12-20181207, Faiz Abbas wrote:
>>>>
>>>>> +
>>>>> +&sdhci0 {
>>>>> +	status = "okay";
>>>>> +	pinctrl-names = "default";
>>>>> +	pinctrl-0 = <&main_mmc0_pins_default>;
>>>>> +	bus-width = <8>;
>>>>> +	non-removable;
>>>>> +	ti,driver-strength-ohm = <50>;
>>>>
>>>> ^^
>>>>
>>>>> +};
>>>>> +
>>>>> +&sdhci1 {
>>>>> +	status = "okay";
>>>>> +	pinctrl-names = "default";
>>>>> +	pinctrl-0 = <&main_mmc1_pins_default>;
>>>>> +	ti,driver-strength-ohm = <50>;
>>>>
>>>> NAK.
>>>>
>>>> $ git checkout next-20181207
>>>> $ git grep ti,driver-strength-ohm Documentation
>>>> $
>>>>
>>>> Nada.. And.. I think "new phy binding" probably introduces this.
>>>> [1] https://patchwork.kernel.org/project/linux-mmc/list/?series=53185
>>>>
>>>> If your patches are'nt really ready, please send them as RFC, I am not
>>>> really in a mood to track the status of every single driver subsystem.
>>>>
>>>> If your binding is not in linux next at the baremin, as far as I am
>>>> concerned, this is not ready, and should be RFC.
>>>
>>> No, RFC does not say "do not merge" or "this has dependencies". RFC is
>>> used to invite a stronger review when introducing a new concept. Its
>>> fair game to apply patches marked RFC if maintainer is okay with the
>>> content.
>>
>> True, fair enough.. RFC is request for comments. Anyways, that is
>> besides the point.
>>>
>>> Dependencies are either noted in cover-letter or below the patch
>>> tear-line. With what you are asking, looks like patches need to be
>>> resubmitted once dependencies are cleared, even if there is no change in
>>> the content itself. This will be additional work.
>>
>> Yes please. There would be other dts changes that are probably ready and
>> I really wont be tracking everything happening on other drivers. If the
>> binding is present at least in next, it is a good indication of things
>> clean and ready to go.
> 
> Agree that bindings should be in linux-next before device-tree files are
> merged.
> 
>>
>>>
>>> That said, if it makes life convenient for you, you can impose such a
>>> rule for patches you need to handle. But I think it will take some
>>> getting used for developers who send patches to you as I don't think
>>> this is a norm elsewhere.
>>>
>>> Adding Tony and Arnd as well, in case I have missed some recently
>>> accepted convention.
>>
>>
>> I have'nt looked at any conventions, The style I prefer to follow when I do
>> submissions: It is my job to get the bindings in, until then my actual
>> dts is just "request for comments". Only after the bindings are merged
>> do I formally submit dts - simply because I dont expect dts maintainer
>> to track what happened to my driver's binding and discussions there of.
> 
> Ok.
> 
>>
>> Seriously, is'nt it really reasonable for dts maintainer to check every
>> single driver's development status in 15 different mailing lists?
>> Because, it sounds like what you are asking. At least I wont have time
>> for it..
>>
>>
>> I really am curious how Arnd / Tony actually pull this one off.. If they
>> have continous cron job for checking if your patch is ready... I doubt
>> it..
> 
> I think you can rely on the author to tell you when something is> actually ready to be merged (and you can tell him/her to remind you).

Yes. I will ping Nishanth once the bindings are in next.

Thanks,
Faiz

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Nishanth Menon]

On 18:10-20181210, Sekhar Nori wrote:
> 
> I think you can rely on the author to tell you when something is
> actually ready to be merged (and you can tell him/her to remind you).
> 
> For the review itself, doing it by having a look at the dependencies
> mentioned in the cover letter (like available for this series) should be
> good enough (I feel).

Dependencies can be for multiple things.. anyways, Sigh..

> I am not sure if there is a need to post an "RFC version", and then
> follow it up with an actual "PATCH version" once dependencies are
> cleared though.
Heads up for the maintainers will be useful. For example:

Will be good to state in cover-letter instead of just stating
https://patchwork.kernel.org/cover/10717641/

State:

Bindings are in discussion[2], will confirm if approved and will
check if rebase is necessary.

This will indicate to the maintainer that patches are preliminary and
subject to change depending on bindings discussion and developer has
taken ownership of maintaining patch sanity.

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Nishanth Menon]

On 19:03-20181210, Faiz Abbas wrote:
> > I think you can rely on the author to tell you when something is> actually ready to be merged (and you can tell him/her to remind you).
> 
> Yes. I will ping Nishanth once the bindings are in next.

In addition, when you are ready, please also check if the patches
need rebase on top of Tero's next branch:
https://git.kernel.org/pub/scm/linux/kernel/git/kristo/linux.git/log/?h=4.20-rc1-am65x-queue

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [181210 12:07]:
> I really am curious how Arnd / Tony actually pull this one off.. If they
> have continous cron job for checking if your patch is ready... I doubt
> it..

The only way that works in a distributed manner is for the patch
authors to resend after the dependencies have cleared.

As some people don't know this, at least I try to reply with
something about tagging patch as read and assuming it will get
resent after dependencies have cleard.

Otherwise the todo list just keeps growing eternally :)

Regards,

Tony

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Vignesh R]

On 07/12/18 2:12 PM, Faiz Abbas wrote:
> On the am654x-evm, sdhci0 node is connected to an eMMC while sdhci1
> is connected to an SD card slot. Add nodes and pinmuxes for the same.
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
>  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> index bd5a0069191d..5dcd16b787eb 100644
> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> @@ -60,6 +60,36 @@
>  			AM65X_IOPAD(0x0070, PIN_INPUT, 5) /* (R25) GPMC0_CSn2.I2C2_SDA */
>  		>;
>  	};
> +
> +	main_mmc0_pins_default: main_mmc0_pins_default {


make dtbs W=12 will warn you:
Character '_' not recommended in node name


> +		pinctrl-single,pins = <
> +			AM65X_IOPAD(0x01a8, PIN_INPUT_PULLDOWN, 0) /* (B25) MMC0_CLK */
> +			AM65X_IOPAD(0x01ac, PIN_INPUT_PULLUP, 0) /* (B27) MMC0_CMD */
> +			AM65X_IOPAD(0x01a4, PIN_INPUT_PULLUP, 0) /* (A26) MMC0_DAT0 */
> +			AM65X_IOPAD(0x01a0, PIN_INPUT_PULLUP, 0) /* (E25) MMC0_DAT1 */
> +			AM65X_IOPAD(0x019c, PIN_INPUT_PULLUP, 0) /* (C26) MMC0_DAT2 */
> +			AM65X_IOPAD(0x0198, PIN_INPUT_PULLUP, 0) /* (A25) MMC0_DAT3 */
> +			AM65X_IOPAD(0x0194, PIN_INPUT_PULLUP, 0) /* (E24) MMC0_DAT4 */
> +			AM65X_IOPAD(0x0190, PIN_INPUT_PULLUP, 0) /* (A24) MMC0_DAT5 */
> +			AM65X_IOPAD(0x018c, PIN_INPUT_PULLUP, 0) /* (B26) MMC0_DAT6 */
> +			AM65X_IOPAD(0x0188, PIN_INPUT_PULLUP, 0) /* (D25) MMC0_DAT7 */
> +			AM65X_IOPAD(0x01b4, PIN_INPUT_PULLUP, 0) /* (A23) MMC0_SDCD */
> +			AM65X_IOPAD(0x01b0, PIN_INPUT, 0) /* (C25) MMC0_DS */
> +		>;
> +	};
> +
> +	main_mmc1_pins_default: main_mmc1_pins_default {

Same here

Regards
Vignesh

> +		pinctrl-single,pins = <
> +			AM65X_IOPAD(0x02d4, PIN_INPUT_PULLDOWN, 0) /* (C27) MMC1_CLK */
> +			AM65X_IOPAD(0x02d8, PIN_INPUT_PULLUP, 0) /* (C28) MMC1_CMD */
> +			AM65X_IOPAD(0x02d0, PIN_INPUT_PULLUP, 0) /* (D28) MMC1_DAT0 */
> +			AM65X_IOPAD(0x02cc, PIN_INPUT_PULLUP, 0) /* (E27) MMC1_DAT1 */
> +			AM65X_IOPAD(0x02c8, PIN_INPUT_PULLUP, 0) /* (D26) MMC1_DAT2 */
> +			AM65X_IOPAD(0x02c4, PIN_INPUT_PULLUP, 0) /* (D27) MMC1_DAT3 */
> +			AM65X_IOPAD(0x02dc, PIN_INPUT_PULLUP, 0) /* (B24) MMC1_SDCD */
> +			AM65X_IOPAD(0x02e0, PIN_INPUT, 0) /* (C24) MMC1_SDWP */
> +		>;
> +	};
>  };
>  
>  &main_pmx1 {
> @@ -125,3 +155,19 @@
>  	pinctrl-0 = <&main_i2c2_pins_default>;
>  	clock-frequency = <400000>;
>  };
> +
> +&sdhci0 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&main_mmc0_pins_default>;
> +	bus-width = <8>;
> +	non-removable;
> +	ti,driver-strength-ohm = <50>;
> +};
> +
> +&sdhci1 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&main_mmc1_pins_default>;
> +	ti,driver-strength-ohm = <50>;
> +};
> 

-- 
Regards
Vignesh

Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: Add MMC/SD support

[Sekhar Nori]

Hi Tony,

On 11/12/18 5:17 AM, Tony Lindgren wrote:
> * Nishanth Menon <nm@ti.com> [181210 12:07]:
>> I really am curious how Arnd / Tony actually pull this one off.. If they
>> have continous cron job for checking if your patch is ready... I doubt
>> it..
> 
> The only way that works in a distributed manner is for the patch
> authors to resend after the dependencies have cleared.
> 
> As some people don't know this, at least I try to reply with
> something about tagging patch as read and assuming it will get
> resent after dependencies have cleard.
> 
> Otherwise the todo list just keeps growing eternally :)

Alright, if the preference is to resend patches after dependencies are
cleared, I think it will also be better to mark the initial review-only
version as "RFC" (as Nishanth requested).

Thanks,
Sekhar