| Message ID | 20220520161004.1141554-2-judyhsiao@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add dtsi for sc7280 herobrine boards that using rt5682 codec | expand |
On Fri, May 20, 2022 at 04:10:02PM +0000, Judy Hsiao wrote: > 1. Add drive strength property for mi2s1 on sc7280 based platforms. > 2. Disbale the pull-up mi2s1_data0, mi2s1_sclk. > > Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Quoting Judy Hsiao (2022-05-20 09:10:02) > 1. Add drive strength property for mi2s1 on sc7280 based platforms. > 2. Disbale the pull-up mi2s1_data0, mi2s1_sclk. s/Disbale/Disable/ The commit text is a list of things done but no reason why they're done. I'd appreciate more freeform text with a blurb why a drive strength is chosen and why pulls are disabled. > Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> > --- > arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > index 9cb1bc8ed6b5..6d8744e130b0 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > @@ -612,6 +612,20 @@ &dp_hot_plug_det { > bias-disable; > }; > > +&mi2s1_data0 { > + drive-strength = <6>; > + bias-disable; Is there an external pull on this line? > +}; > + > +&mi2s1_sclk { > + drive-strength = <6>; > + bias-disable; Is there an external pull on this line? If so please add that details as a comment like we do for other external pulls. > +};
Hi, On Fri, May 20, 2022 at 1:38 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Judy Hsiao (2022-05-20 09:10:02) > > 1. Add drive strength property for mi2s1 on sc7280 based platforms. > > 2. Disbale the pull-up mi2s1_data0, mi2s1_sclk. > > s/Disbale/Disable/ > > The commit text is a list of things done but no reason why they're done. > I'd appreciate more freeform text with a blurb why a drive strength is > chosen and why pulls are disabled. > > > Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> > > --- > > arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > > index 9cb1bc8ed6b5..6d8744e130b0 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > > @@ -612,6 +612,20 @@ &dp_hot_plug_det { > > bias-disable; > > }; > > > > +&mi2s1_data0 { > > + drive-strength = <6>; > > + bias-disable; > > Is there an external pull on this line? > > > +}; > > + > > +&mi2s1_sclk { > > + drive-strength = <6>; > > + bias-disable; > > Is there an external pull on this line? If so please add that details as > a comment like we do for other external pulls. Actually, I think they are output lines, which is why they have a drive-strength. I think for output lines we don't usually comment about why we're disabling the pulls, only for input lines? -Doug
Hi, On Fri, May 20, 2022 at 9:10 AM Judy Hsiao <judyhsiao@chromium.org> wrote: > > 1. Add drive strength property for mi2s1 on sc7280 based platforms. > 2. Disbale the pull-up mi2s1_data0, mi2s1_sclk. > > Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> > --- > arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > index 9cb1bc8ed6b5..6d8744e130b0 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > @@ -612,6 +612,20 @@ &dp_hot_plug_det { > bias-disable; > }; > > +&mi2s1_data0 { > + drive-strength = <6>; > + bias-disable; > +}; > + > +&mi2s1_sclk { > + drive-strength = <6>; > + bias-disable; > +}; > + > +&mi2s1_ws { > + drive-strength = <6>; > +}; I'm actually curious why this line has a drive-strength but _no_ bias setting. I guess I should figure out what the heck "ws" is. Ah, I guess it is word select. Since this is an output I'd expect to see "bias-disable" here too. -Doug
Quoting Doug Anderson (2022-05-20 13:39:21) > On Fri, May 20, 2022 at 1:38 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > Quoting Judy Hsiao (2022-05-20 09:10:02) > > > +}; > > > + > > > +&mi2s1_sclk { > > > + drive-strength = <6>; > > > + bias-disable; > > > > Is there an external pull on this line? If so please add that details as > > a comment like we do for other external pulls. > > Actually, I think they are output lines, which is why they have a > drive-strength. I think for output lines we don't usually comment > about why we're disabling the pulls, only for input lines? Ok makes sense. Even for an open drain signal it would be an "input" so that rule still applies?
Hi, On Fri, May 20, 2022 at 2:01 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Doug Anderson (2022-05-20 13:39:21) > > On Fri, May 20, 2022 at 1:38 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > > > Quoting Judy Hsiao (2022-05-20 09:10:02) > > > > +}; > > > > + > > > > +&mi2s1_sclk { > > > > + drive-strength = <6>; > > > > + bias-disable; > > > > > > Is there an external pull on this line? If so please add that details as > > > a comment like we do for other external pulls. > > > > Actually, I think they are output lines, which is why they have a > > drive-strength. I think for output lines we don't usually comment > > about why we're disabling the pulls, only for input lines? > > Ok makes sense. Even for an open drain signal it would be an "input" so > that rule still applies? I think open drain is mostly used for bidirectional signals, like i2c lines. In that case then you're right, you can have a drive-strength and a pull. ...I thought i2s was not bidirectoinal and not open-drain, but I certainly could be wrong. -Doug
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi index 9cb1bc8ed6b5..6d8744e130b0 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi @@ -612,6 +612,20 @@ &dp_hot_plug_det { bias-disable; }; +&mi2s1_data0 { + drive-strength = <6>; + bias-disable; +}; + +&mi2s1_sclk { + drive-strength = <6>; + bias-disable; +}; + +&mi2s1_ws { + drive-strength = <6>; +}; + &pcie1_clkreq_n { bias-pull-up; drive-strength = <2>;
1. Add drive strength property for mi2s1 on sc7280 based platforms. 2. Disbale the pull-up mi2s1_data0, mi2s1_sclk. Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> --- arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)