| Message ID | 20230203-evk-board-support-v2-3-6ec7cdb10ccf@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve the MT8365 SoC and EVK board support | expand |
On Tue, Mar 7, 2023 at 2:18 PM Alexandre Mergnat <amergnat@baylibre.com> wrote: > This SoC is able to drive the following output current: > - 2 mA > - 4 mA > - 6 mA > - 8 mA > - 10 mA > - 12 mA > - 14 mA > - 16 mA > > Then drive-strength property is set with enum to reflect its HW capability. > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> Is this something I can apply directly to the pinctrl tree once the DT maintainers have had a chance to look at it? Yours, Linus Walleij
On 07/03/2023 14:17, Alexandre Mergnat wrote: > This SoC is able to drive the following output current: > - 2 mA > - 4 mA > - 6 mA > - 8 mA > - 10 mA > - 12 mA > - 14 mA > - 16 mA > > Then drive-strength property is set with enum to reflect its HW capability. > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- > Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml > index 4b96884a1afc..101871ec6693 100644 > --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml > +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml > @@ -79,6 +79,9 @@ patternProperties: > > bias-pull-down: true > > + drive-strength: > + enum: [2, 4, 6, 8, 10, 12, 14, 16] Isn't this conflicting with mediatek,drive-strength-adv? Your commit msg suggests you add a missing property, but I would say nothing was missing here. You need review from (pinctrl) Mediatek maintainers how the bindings for all Mediateks are organized. Best regards, Krzysztof
Il 10/03/23 09:32, Krzysztof Kozlowski ha scritto: > On 07/03/2023 14:17, Alexandre Mergnat wrote: >> This SoC is able to drive the following output current: >> - 2 mA >> - 4 mA >> - 6 mA >> - 8 mA >> - 10 mA >> - 12 mA >> - 14 mA >> - 16 mA >> >> Then drive-strength property is set with enum to reflect its HW capability. >> >> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >> --- >> Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml >> index 4b96884a1afc..101871ec6693 100644 >> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml >> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml >> @@ -79,6 +79,9 @@ patternProperties: >> >> bias-pull-down: true >> >> + drive-strength: >> + enum: [2, 4, 6, 8, 10, 12, 14, 16] > > Isn't this conflicting with mediatek,drive-strength-adv? Your commit msg > suggests you add a missing property, but I would say nothing was missing > here. > > You need review from (pinctrl) Mediatek maintainers how the bindings for > all Mediateks are organized. Hello Krzysztof, mediatek,drive-strength-adv *shall not exist*, that was an unnecessary property that leaked upstream from downstream kernels and there's no reason to use it. Upstream, we have drive-strength-microamp and mediatek,rsel-resistance-in-si-unit. Since mediatek,mt8365-pinctrl.yaml got picked with that property already, I have nothing to complain about this specific commit... drive-strength does not conflict with the mediatek,drive-strength-adv property, as the "adv" is for microamp adjustments. You can pick it, it's fine. Anyway, Alexandre: can you please perform a cleanup to the MT8365 pinctrl binding? The cleanup means you're setting mediatek,drive-strength-adv as deprecated and adding the right properties (...and possibly changing the devicetrees to use it). For more information, you can look at commit history for the (unfortunately, named incorrectly) MT8195 pinctrl documentation: bindings/pinctrl/pinctrl-mt8195.yaml where we performed the same cleanup that I'm asking you to do, except we didn't have to set any property as deprecated because there was *no devicetree upstream* that was actually using that property (hence not an ABI breakage). Cheers! Angelo
Il 10/03/23 10:46, AngeloGioacchino Del Regno ha scritto: > Il 10/03/23 09:32, Krzysztof Kozlowski ha scritto: >> On 07/03/2023 14:17, Alexandre Mergnat wrote: >>> This SoC is able to drive the following output current: >>> - 2 mA >>> - 4 mA >>> - 6 mA >>> - 8 mA >>> - 10 mA >>> - 12 mA >>> - 14 mA >>> - 16 mA >>> >>> Then drive-strength property is set with enum to reflect its HW capability. >>> >>> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >>> --- >>> Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml >>> b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml >>> index 4b96884a1afc..101871ec6693 100644 >>> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml >>> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml >>> @@ -79,6 +79,9 @@ patternProperties: >>> bias-pull-down: true >>> + drive-strength: >>> + enum: [2, 4, 6, 8, 10, 12, 14, 16] >> >> Isn't this conflicting with mediatek,drive-strength-adv? Your commit msg >> suggests you add a missing property, but I would say nothing was missing >> here. >> >> You need review from (pinctrl) Mediatek maintainers how the bindings for >> all Mediateks are organized. > > Hello Krzysztof, > > mediatek,drive-strength-adv *shall not exist*, that was an unnecessary property > that leaked upstream from downstream kernels and there's no reason to use it. > > Upstream, we have drive-strength-microamp and mediatek,rsel-resistance-in-si-unit. > > Since mediatek,mt8365-pinctrl.yaml got picked with that property already, I have > nothing to complain about this specific commit... drive-strength does not conflict > with the mediatek,drive-strength-adv property, as the "adv" is for microamp > adjustments. > > You can pick it, it's fine. > > Anyway, Alexandre: can you please perform a cleanup to the MT8365 pinctrl binding? > The cleanup means you're setting mediatek,drive-strength-adv as deprecated and > adding the right properties (...and possibly changing the devicetrees to use it). > > For more information, you can look at commit history for the (unfortunately, named > incorrectly) MT8195 pinctrl documentation: bindings/pinctrl/pinctrl-mt8195.yaml > where we performed the same cleanup that I'm asking you to do, except we didn't > have to set any property as deprecated because there was *no devicetree upstream* > that was actually using that property (hence not an ABI breakage). > > Cheers! > Angelo Sorry for the double email. I forgot to give my: Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Whoops! Cheers again :-)
Le ven. 10 mars 2023 à 10:49, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> a écrit : > > Il 10/03/23 10:46, AngeloGioacchino Del Regno ha scritto: > > > > Anyway, Alexandre: can you please perform a cleanup to the MT8365 pinctrl binding? Yes I can ! :D Should I do it directly in this patch series or (I prefer) in a new one ? > > The cleanup means you're setting mediatek,drive-strength-adv as deprecated and > > adding the right properties (...and possibly changing the devicetrees to use it). > > > > For more information, you can look at commit history for the (unfortunately, named > > incorrectly) MT8195 pinctrl documentation: bindings/pinctrl/pinctrl-mt8195.yaml > > where we performed the same cleanup that I'm asking you to do, except we didn't > > have to set any property as deprecated because there was *no devicetree upstream* > > that was actually using that property (hence not an ABI breakage). Thanks for the information, that helps. Regards, Alex
Il 15/03/23 09:11, Alexandre Mergnat ha scritto: > Le ven. 10 mars 2023 à 10:49, AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> a écrit : >> >> Il 10/03/23 10:46, AngeloGioacchino Del Regno ha scritto: >>> >>> Anyway, Alexandre: can you please perform a cleanup to the MT8365 pinctrl binding? > > Yes I can ! :D > Should I do it directly in this patch series or (I prefer) in a new one ? > Doing that in a new one is fine... it's a cleanup that is not strictly related to what you're introducing in this series. >>> The cleanup means you're setting mediatek,drive-strength-adv as deprecated and >>> adding the right properties (...and possibly changing the devicetrees to use it). >>> >>> For more information, you can look at commit history for the (unfortunately, named >>> incorrectly) MT8195 pinctrl documentation: bindings/pinctrl/pinctrl-mt8195.yaml >>> where we performed the same cleanup that I'm asking you to do, except we didn't >>> have to set any property as deprecated because there was *no devicetree upstream* >>> that was actually using that property (hence not an ABI breakage). > > Thanks for the information, that helps. You're welcome! Regards, Angelo
diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml index 4b96884a1afc..101871ec6693 100644 --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml @@ -79,6 +79,9 @@ patternProperties: bias-pull-down: true + drive-strength: + enum: [2, 4, 6, 8, 10, 12, 14, 16] + input-enable: true input-disable: true
This SoC is able to drive the following output current: - 2 mA - 4 mA - 6 mA - 8 mA - 10 mA - 12 mA - 14 mA - 16 mA Then drive-strength property is set with enum to reflect its HW capability. Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> --- Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml | 3 +++ 1 file changed, 3 insertions(+)