* [PATCH 0/3] phy: ti: j721e-wiz: Add support for DisplayPort mode @ 2019-12-09 16:20 Jyri Sarha 2019-12-09 16:21 ` [PATCH 1/3] dt-bindings: phy: Add PHY_TYPE_DP definition Jyri Sarha ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Jyri Sarha @ 2019-12-09 16:20 UTC (permalink / raw) To: kishon, linux-kernel, devicetree Cc: tomi.valkeinen, praneeth, yamonkar, sjakhade, rogerq, jsarha The wiz wrapper need slightly different configuration when the wrapped serdes is used for DisplayPort. This series adds devicetree properties for selecting the mode for each individual serdes lane separately. The patch series should apply on top of these series: https://lkml.org/lkml/2019/10/23/589 https://lkml.org/lkml/2019/10/28/197 Jyri Sarha (3): dt-bindings: phy: Add PHY_TYPE_DP definition dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver .../bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++ drivers/phy/ti/phy-j721e-wiz.c | 55 +++++++++++++++++-- include/dt-bindings/phy/phy.h | 1 + 3 files changed, 64 insertions(+), 4 deletions(-) -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] dt-bindings: phy: Add PHY_TYPE_DP definition 2019-12-09 16:20 [PATCH 0/3] phy: ti: j721e-wiz: Add support for DisplayPort mode Jyri Sarha @ 2019-12-09 16:21 ` Jyri Sarha 2019-12-19 19:04 ` Rob Herring 2019-12-09 16:22 ` [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) Jyri Sarha 2019-12-09 16:22 ` [PATCH 3/3] phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver Jyri Sarha 2 siblings, 1 reply; 12+ messages in thread From: Jyri Sarha @ 2019-12-09 16:21 UTC (permalink / raw) To: kishon, linux-kernel, devicetree Cc: tomi.valkeinen, praneeth, yamonkar, sjakhade, rogerq, jsarha Add definition for DisplayPort phy type. Signed-off-by: Jyri Sarha <jsarha@ti.com> Reviewed-by: Roger Quadros <rogerq@ti.com> Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com> --- include/dt-bindings/phy/phy.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h index b6a1eaf1b339..1f3f866fae7b 100644 --- a/include/dt-bindings/phy/phy.h +++ b/include/dt-bindings/phy/phy.h @@ -16,5 +16,6 @@ #define PHY_TYPE_USB2 3 #define PHY_TYPE_USB3 4 #define PHY_TYPE_UFS 5 +#define PHY_TYPE_DP 6 #endif /* _DT_BINDINGS_PHY */ -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] dt-bindings: phy: Add PHY_TYPE_DP definition 2019-12-09 16:21 ` [PATCH 1/3] dt-bindings: phy: Add PHY_TYPE_DP definition Jyri Sarha @ 2019-12-19 19:04 ` Rob Herring 0 siblings, 0 replies; 12+ messages in thread From: Rob Herring @ 2019-12-19 19:04 UTC (permalink / raw) To: Jyri Sarha Cc: kishon, linux-kernel, devicetree, tomi.valkeinen, praneeth, yamonkar, sjakhade, rogerq, jsarha On Mon, 9 Dec 2019 18:21:09 +0200, Jyri Sarha wrote: > Add definition for DisplayPort phy type. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > Reviewed-by: Roger Quadros <rogerq@ti.com> > Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > include/dt-bindings/phy/phy.h | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-09 16:20 [PATCH 0/3] phy: ti: j721e-wiz: Add support for DisplayPort mode Jyri Sarha 2019-12-09 16:21 ` [PATCH 1/3] dt-bindings: phy: Add PHY_TYPE_DP definition Jyri Sarha @ 2019-12-09 16:22 ` Jyri Sarha 2019-12-19 19:08 ` Rob Herring 2019-12-09 16:22 ` [PATCH 3/3] phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver Jyri Sarha 2 siblings, 1 reply; 12+ messages in thread From: Jyri Sarha @ 2019-12-09 16:22 UTC (permalink / raw) To: kishon, linux-kernel, devicetree, robh+dt Cc: tomi.valkeinen, praneeth, yamonkar, sjakhade, rogerq, jsarha Add property to indicate the usage of SERDES lane controlled by the WIZ wrapper. The wrapper configuration has some variation depending on how each lane is going to be used. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml index 94e3b4b5ed8e..399725f65278 100644 --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml @@ -97,6 +97,18 @@ patternProperties: Torrent SERDES should follow the bindings specified in Documentation/devicetree/bindings/phy/phy-cadence-dp.txt + "^lane[1-4]-mode$": + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: [0, 1, 2, 3, 4, 5, 6] + description: | + Integer describing static lane usage for the lane indicated in + the property name. For Sierra there may be properties lane0 and + lane1, for Torrent all lane[1-4]-mode properties may be + there. The constants to indicate the lane usage are defined in + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused + if its lane<n>-use property does not exist. + required: - compatible - power-domains -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-09 16:22 ` [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) Jyri Sarha @ 2019-12-19 19:08 ` Rob Herring 2019-12-20 12:52 ` Jyri Sarha 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2019-12-19 19:08 UTC (permalink / raw) To: Jyri Sarha Cc: kishon, linux-kernel, devicetree, tomi.valkeinen, praneeth, yamonkar, sjakhade, rogerq On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote: > Add property to indicate the usage of SERDES lane controlled by the > WIZ wrapper. The wrapper configuration has some variation depending on > how each lane is going to be used. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > index 94e3b4b5ed8e..399725f65278 100644 > --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > @@ -97,6 +97,18 @@ patternProperties: > Torrent SERDES should follow the bindings specified in > Documentation/devicetree/bindings/phy/phy-cadence-dp.txt > > + "^lane[1-4]-mode$": > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [0, 1, 2, 3, 4, 5, 6] > + description: | > + Integer describing static lane usage for the lane indicated in > + the property name. For Sierra there may be properties lane0 and > + lane1, for Torrent all lane[1-4]-mode properties may be > + there. The constants to indicate the lane usage are defined in > + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused > + if its lane<n>-use property does not exist. The defines were intended to be in 'phys' cells. Does putting both lane and mode in the client 'phys' properties not work? Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-19 19:08 ` Rob Herring @ 2019-12-20 12:52 ` Jyri Sarha 2019-12-24 21:31 ` Rob Herring 0 siblings, 1 reply; 12+ messages in thread From: Jyri Sarha @ 2019-12-20 12:52 UTC (permalink / raw) To: Rob Herring Cc: kishon, linux-kernel, devicetree, tomi.valkeinen, praneeth, yamonkar, sjakhade, rogerq On 19/12/2019 21:08, Rob Herring wrote: > On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote: >> Add property to indicate the usage of SERDES lane controlled by the >> WIZ wrapper. The wrapper configuration has some variation depending on >> how each lane is going to be used. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >> index 94e3b4b5ed8e..399725f65278 100644 >> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >> @@ -97,6 +97,18 @@ patternProperties: >> Torrent SERDES should follow the bindings specified in >> Documentation/devicetree/bindings/phy/phy-cadence-dp.txt >> >> + "^lane[1-4]-mode$": >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + - enum: [0, 1, 2, 3, 4, 5, 6] >> + description: | >> + Integer describing static lane usage for the lane indicated in >> + the property name. For Sierra there may be properties lane0 and >> + lane1, for Torrent all lane[1-4]-mode properties may be >> + there. The constants to indicate the lane usage are defined in >> + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused >> + if its lane<n>-use property does not exist. > > The defines were intended to be in 'phys' cells. Does putting both lane > and mode in the client 'phys' properties not work? > Let me first check if I understood you. So you are suggesting something like this: dp-phy { #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */ ... }; dp-bridge { ... phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */ }; Best regards, Jyri -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-20 12:52 ` Jyri Sarha @ 2019-12-24 21:31 ` Rob Herring 2019-12-30 9:37 ` Jyri Sarha 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2019-12-24 21:31 UTC (permalink / raw) To: Jyri Sarha Cc: Kishon Vijay Abraham I, linux-kernel, devicetree, Tomi Valkeinen, Praneeth Bajjuri, Yuti Amonkar, Swapnil Kashinath Jakhade, Roger Quadros On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote: > > On 19/12/2019 21:08, Rob Herring wrote: > > On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote: > >> Add property to indicate the usage of SERDES lane controlled by the > >> WIZ wrapper. The wrapper configuration has some variation depending on > >> how each lane is going to be used. > >> > >> Signed-off-by: Jyri Sarha <jsarha@ti.com> > >> --- > >> .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ > >> 1 file changed, 12 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > >> index 94e3b4b5ed8e..399725f65278 100644 > >> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > >> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > >> @@ -97,6 +97,18 @@ patternProperties: > >> Torrent SERDES should follow the bindings specified in > >> Documentation/devicetree/bindings/phy/phy-cadence-dp.txt > >> > >> + "^lane[1-4]-mode$": > >> + allOf: > >> + - $ref: /schemas/types.yaml#/definitions/uint32 > >> + - enum: [0, 1, 2, 3, 4, 5, 6] > >> + description: | > >> + Integer describing static lane usage for the lane indicated in > >> + the property name. For Sierra there may be properties lane0 and > >> + lane1, for Torrent all lane[1-4]-mode properties may be > >> + there. The constants to indicate the lane usage are defined in > >> + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused > >> + if its lane<n>-use property does not exist. > > > > The defines were intended to be in 'phys' cells. Does putting both lane > > and mode in the client 'phys' properties not work? > > > > Let me first check if I understood you. So you are suggesting something > like this: > > dp-phy { > #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */ > ... > }; > > dp-bridge { > ... > phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */ Yes, but I think the lanes can be a single cell mask. And I'd probably make that the first cell which is generally "which PHY" and make type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines and match what they've done if possible. Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-24 21:31 ` Rob Herring @ 2019-12-30 9:37 ` Jyri Sarha 2019-12-30 10:18 ` Kishon Vijay Abraham I 2019-12-30 18:21 ` Rob Herring 0 siblings, 2 replies; 12+ messages in thread From: Jyri Sarha @ 2019-12-30 9:37 UTC (permalink / raw) To: Rob Herring, Kishon Vijay Abraham I Cc: linux-kernel, devicetree, Tomi Valkeinen, Praneeth Bajjuri, Yuti Amonkar, Swapnil Kashinath Jakhade, Roger Quadros On 24/12/2019 23:31, Rob Herring wrote: > On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote: >> >> On 19/12/2019 21:08, Rob Herring wrote: >>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote: >>>> Add property to indicate the usage of SERDES lane controlled by the >>>> WIZ wrapper. The wrapper configuration has some variation depending on >>>> how each lane is going to be used. >>>> >>>> Signed-off-by: Jyri Sarha <jsarha@ti.com> >>>> --- >>>> .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ >>>> 1 file changed, 12 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>> index 94e3b4b5ed8e..399725f65278 100644 >>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>> @@ -97,6 +97,18 @@ patternProperties: >>>> Torrent SERDES should follow the bindings specified in >>>> Documentation/devicetree/bindings/phy/phy-cadence-dp.txt >>>> >>>> + "^lane[1-4]-mode$": >>>> + allOf: >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>> + - enum: [0, 1, 2, 3, 4, 5, 6] >>>> + description: | >>>> + Integer describing static lane usage for the lane indicated in >>>> + the property name. For Sierra there may be properties lane0 and >>>> + lane1, for Torrent all lane[1-4]-mode properties may be >>>> + there. The constants to indicate the lane usage are defined in >>>> + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused >>>> + if its lane<n>-use property does not exist. >>> >>> The defines were intended to be in 'phys' cells. Does putting both lane >>> and mode in the client 'phys' properties not work? >>> >> >> Let me first check if I understood you. So you are suggesting something >> like this: >> >> dp-phy { >> #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */ >> ... >> }; >> >> dp-bridge { >> ... >> phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */ > > Yes, but I think the lanes can be a single cell mask. And I'd probably > make that the first cell which is generally "which PHY" and make > type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines > and match what they've done if possible. > I see. This will cause some head ache on the driver implementation side, as there is no way for the phy driver to peek the lane use or type from the phy client's device tree node. It also looks to me that the phy API[1] has to be extended quite a bit before the phy client can pass the lane usage information to the phy driver. It will cause some pain to implement the extension without breaking the phy API and causing a nasty cross dependency over all the phy client domains. Also, there is not much point in putting the PHY_TYPE constant to the phy client's node, as normally the phy client driver will know quite well what PHY_TYPE to use. E.g. a SATA driver will always select PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc. Kishon, if we have to take this road it also starts to sound like we will have to move the phy client's phandle to point to the phy wrapper node, if we want to keep the actual phy driver wrapper agnostic. Then we can make the wrapper to act like a proxy that forwards the phy_ops calls to the actual phy driver. Luckily the per lane phy-type selection is not a blocker for our j721e DisplayPort functionality. Best regards, Jyri [1] include/linux/phy/phy.h -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-30 9:37 ` Jyri Sarha @ 2019-12-30 10:18 ` Kishon Vijay Abraham I 2019-12-30 11:54 ` Jyri Sarha 2019-12-30 18:21 ` Rob Herring 1 sibling, 1 reply; 12+ messages in thread From: Kishon Vijay Abraham I @ 2019-12-30 10:18 UTC (permalink / raw) To: Jyri Sarha, Rob Herring Cc: linux-kernel, devicetree, Tomi Valkeinen, Praneeth Bajjuri, Yuti Amonkar, Swapnil Kashinath Jakhade, Roger Quadros Hi, On 30/12/19 3:07 PM, Jyri Sarha wrote: > On 24/12/2019 23:31, Rob Herring wrote: >> On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote: >>> >>> On 19/12/2019 21:08, Rob Herring wrote: >>>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote: >>>>> Add property to indicate the usage of SERDES lane controlled by the >>>>> WIZ wrapper. The wrapper configuration has some variation depending on >>>>> how each lane is going to be used. >>>>> >>>>> Signed-off-by: Jyri Sarha <jsarha@ti.com> >>>>> --- >>>>> .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ >>>>> 1 file changed, 12 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>>> index 94e3b4b5ed8e..399725f65278 100644 >>>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>>> @@ -97,6 +97,18 @@ patternProperties: >>>>> Torrent SERDES should follow the bindings specified in >>>>> Documentation/devicetree/bindings/phy/phy-cadence-dp.txt >>>>> >>>>> + "^lane[1-4]-mode$": >>>>> + allOf: >>>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + - enum: [0, 1, 2, 3, 4, 5, 6] >>>>> + description: | >>>>> + Integer describing static lane usage for the lane indicated in >>>>> + the property name. For Sierra there may be properties lane0 and >>>>> + lane1, for Torrent all lane[1-4]-mode properties may be >>>>> + there. The constants to indicate the lane usage are defined in >>>>> + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused >>>>> + if its lane<n>-use property does not exist. >>>> >>>> The defines were intended to be in 'phys' cells. Does putting both lane >>>> and mode in the client 'phys' properties not work? >>>> >>> >>> Let me first check if I understood you. So you are suggesting something >>> like this: >>> >>> dp-phy { >>> #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */ >>> ... >>> }; >>> >>> dp-bridge { >>> ... >>> phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */ >> >> Yes, but I think the lanes can be a single cell mask. And I'd probably >> make that the first cell which is generally "which PHY" and make >> type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines >> and match what they've done if possible. >> > > I see. This will cause some head ache on the driver implementation side, > as there is no way for the phy driver to peek the lane use or type from > the phy client's device tree node. It also looks to me that the phy > API[1] has to be extended quite a bit before the phy client can pass the > lane usage information to the phy driver. It will cause some pain to > implement the extension without breaking the phy API and causing a nasty > cross dependency over all the phy client domains. > > Also, there is not much point in putting the PHY_TYPE constant to the > phy client's node, as normally the phy client driver will know quite > well what PHY_TYPE to use. E.g. a SATA driver will always select > PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc. > > Kishon, if we have to take this road it also starts to sound like we > will have to move the phy client's phandle to point to the phy wrapper > node, if we want to keep the actual phy driver wrapper agnostic. Then we > can make the wrapper to act like a proxy that forwards the phy_ops calls > to the actual phy driver. Luckily the per lane phy-type selection is not > a blocker for our j721e DisplayPort functionality. WIZ is a PHY wrapper and not a PHY in itself. I'm not inclined in modeling WIZ as a PHY and adding an additional level of indirection. This can add more challenges w.r.t PHY sequencing and can also lead to locking issues. That also doesn't accurately represent the HW bock. Thanks Kishon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-30 10:18 ` Kishon Vijay Abraham I @ 2019-12-30 11:54 ` Jyri Sarha 0 siblings, 0 replies; 12+ messages in thread From: Jyri Sarha @ 2019-12-30 11:54 UTC (permalink / raw) To: Kishon Vijay Abraham I, Rob Herring Cc: linux-kernel, devicetree, Tomi Valkeinen, Praneeth Bajjuri, Yuti Amonkar, Swapnil Kashinath Jakhade, Roger Quadros On 30/12/2019 12:18, Kishon Vijay Abraham I wrote: > Hi, > > On 30/12/19 3:07 PM, Jyri Sarha wrote: >> On 24/12/2019 23:31, Rob Herring wrote: >>> On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote: >>>> >>>> On 19/12/2019 21:08, Rob Herring wrote: >>>>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote: >>>>>> Add property to indicate the usage of SERDES lane controlled by the >>>>>> WIZ wrapper. The wrapper configuration has some variation depending on >>>>>> how each lane is going to be used. >>>>>> >>>>>> Signed-off-by: Jyri Sarha <jsarha@ti.com> >>>>>> --- >>>>>> .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ >>>>>> 1 file changed, 12 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>>>> index 94e3b4b5ed8e..399725f65278 100644 >>>>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>>>> @@ -97,6 +97,18 @@ patternProperties: >>>>>> Torrent SERDES should follow the bindings specified in >>>>>> Documentation/devicetree/bindings/phy/phy-cadence-dp.txt >>>>>> >>>>>> + "^lane[1-4]-mode$": >>>>>> + allOf: >>>>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>>>> + - enum: [0, 1, 2, 3, 4, 5, 6] >>>>>> + description: | >>>>>> + Integer describing static lane usage for the lane indicated in >>>>>> + the property name. For Sierra there may be properties lane0 and >>>>>> + lane1, for Torrent all lane[1-4]-mode properties may be >>>>>> + there. The constants to indicate the lane usage are defined in >>>>>> + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused >>>>>> + if its lane<n>-use property does not exist. >>>>> >>>>> The defines were intended to be in 'phys' cells. Does putting both lane >>>>> and mode in the client 'phys' properties not work? >>>>> >>>> >>>> Let me first check if I understood you. So you are suggesting something >>>> like this: >>>> >>>> dp-phy { >>>> #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */ >>>> ... >>>> }; >>>> >>>> dp-bridge { >>>> ... >>>> phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */ >>> >>> Yes, but I think the lanes can be a single cell mask. And I'd probably >>> make that the first cell which is generally "which PHY" and make >>> type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines >>> and match what they've done if possible. >>> >> >> I see. This will cause some head ache on the driver implementation side, >> as there is no way for the phy driver to peek the lane use or type from >> the phy client's device tree node. It also looks to me that the phy >> API[1] has to be extended quite a bit before the phy client can pass the >> lane usage information to the phy driver. It will cause some pain to >> implement the extension without breaking the phy API and causing a nasty >> cross dependency over all the phy client domains. >> >> Also, there is not much point in putting the PHY_TYPE constant to the >> phy client's node, as normally the phy client driver will know quite >> well what PHY_TYPE to use. E.g. a SATA driver will always select >> PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc. >> >> Kishon, if we have to take this road it also starts to sound like we >> will have to move the phy client's phandle to point to the phy wrapper >> node, if we want to keep the actual phy driver wrapper agnostic. Then we >> can make the wrapper to act like a proxy that forwards the phy_ops calls >> to the actual phy driver. Luckily the per lane phy-type selection is not >> a blocker for our j721e DisplayPort functionality. > > WIZ is a PHY wrapper and not a PHY in itself. I'm not inclined in > modeling WIZ as a PHY and adding an additional level of indirection. > This can add more challenges w.r.t PHY sequencing and can also lead to > locking issues. That also doesn't accurately represent the HW bock. > Ok, then assuming the phy wrapper node's lane<n>-mode property can't be used and if the lane-mode information is only available at the phy client driver, we must somehow deliver the phy-mode information from the phy client driver to the phy wrapper driver. Maybe a way for the phy wrapper driver to request the phy-mode from the actual phy driver, which in turn gets it from the phy client through phy_ops set_mode() call-back. Then there is the extra twist of a single phy driver serving multiple phy clients using different lanes, but we do not need to cross that bridge for the current DisplayPort functionality. Best regards, Jyri -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) 2019-12-30 9:37 ` Jyri Sarha 2019-12-30 10:18 ` Kishon Vijay Abraham I @ 2019-12-30 18:21 ` Rob Herring 1 sibling, 0 replies; 12+ messages in thread From: Rob Herring @ 2019-12-30 18:21 UTC (permalink / raw) To: Jyri Sarha Cc: Kishon Vijay Abraham I, linux-kernel, devicetree, Tomi Valkeinen, Praneeth Bajjuri, Yuti Amonkar, Swapnil Kashinath Jakhade, Roger Quadros On Mon, Dec 30, 2019 at 2:37 AM Jyri Sarha <jsarha@ti.com> wrote: > > On 24/12/2019 23:31, Rob Herring wrote: > > On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote: > >> > >> On 19/12/2019 21:08, Rob Herring wrote: > >>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote: > >>>> Add property to indicate the usage of SERDES lane controlled by the > >>>> WIZ wrapper. The wrapper configuration has some variation depending on > >>>> how each lane is going to be used. > >>>> > >>>> Signed-off-by: Jyri Sarha <jsarha@ti.com> > >>>> --- > >>>> .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml | 12 ++++++++++++ > >>>> 1 file changed, 12 insertions(+) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > >>>> index 94e3b4b5ed8e..399725f65278 100644 > >>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > >>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > >>>> @@ -97,6 +97,18 @@ patternProperties: > >>>> Torrent SERDES should follow the bindings specified in > >>>> Documentation/devicetree/bindings/phy/phy-cadence-dp.txt > >>>> > >>>> + "^lane[1-4]-mode$": > >>>> + allOf: > >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 > >>>> + - enum: [0, 1, 2, 3, 4, 5, 6] > >>>> + description: | > >>>> + Integer describing static lane usage for the lane indicated in > >>>> + the property name. For Sierra there may be properties lane0 and > >>>> + lane1, for Torrent all lane[1-4]-mode properties may be > >>>> + there. The constants to indicate the lane usage are defined in > >>>> + "include/dt-bindings/phy/phy.h". The lane is assumed to be unused > >>>> + if its lane<n>-use property does not exist. > >>> > >>> The defines were intended to be in 'phys' cells. Does putting both lane > >>> and mode in the client 'phys' properties not work? > >>> > >> > >> Let me first check if I understood you. So you are suggesting something > >> like this: > >> > >> dp-phy { > >> #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */ > >> ... > >> }; > >> > >> dp-bridge { > >> ... > >> phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */ > > > > Yes, but I think the lanes can be a single cell mask. And I'd probably > > make that the first cell which is generally "which PHY" and make > > type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines > > and match what they've done if possible. > > > > I see. This will cause some head ache on the driver implementation side, > as there is no way for the phy driver to peek the lane use or type from > the phy client's device tree node. Yes, there is a way. Not really fast, but use for_each_node_with_property(node, "phys") and filter on ones matching your phy's node. > It also looks to me that the phy > API[1] has to be extended quite a bit before the phy client can pass the > lane usage information to the phy driver. It will cause some pain to > implement the extension without breaking the phy API and causing a nasty > cross dependency over all the phy client domains. Not really a concern from a binding standpoint. Bindings shouldn't be designed around some OS's current design or limitations. There's already several cases using PHY_TYPE_* in phy cells, so I'm not sure what the issue is. > Also, there is not much point in putting the PHY_TYPE constant to the > phy client's node, as normally the phy client driver will know quite > well what PHY_TYPE to use. E.g. a SATA driver will always select > PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc. Good point. That could work as well. > Kishon, if we have to take this road it also starts to sound like we > will have to move the phy client's phandle to point to the phy wrapper > node, if we want to keep the actual phy driver wrapper agnostic. Then we > can make the wrapper to act like a proxy that forwards the phy_ops calls > to the actual phy driver. Luckily the per lane phy-type selection is not > a blocker for our j721e DisplayPort functionality. > > Best regards, > Jyri > > [1] include/linux/phy/phy.h > > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver 2019-12-09 16:20 [PATCH 0/3] phy: ti: j721e-wiz: Add support for DisplayPort mode Jyri Sarha 2019-12-09 16:21 ` [PATCH 1/3] dt-bindings: phy: Add PHY_TYPE_DP definition Jyri Sarha 2019-12-09 16:22 ` [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) Jyri Sarha @ 2019-12-09 16:22 ` Jyri Sarha 2 siblings, 0 replies; 12+ messages in thread From: Jyri Sarha @ 2019-12-09 16:22 UTC (permalink / raw) To: kishon, linux-kernel, devicetree Cc: tomi.valkeinen, praneeth, yamonkar, sjakhade, rogerq, jsarha For DisplayPort use we need to set WIZ_CONFIG_LANECTL register's P_STANDARD_MODE bits to "mode 3". In the DisplayPort use also the P_ENABLE bits of the same register are set to P_ENABLE instead of P_ENABLE_FORCE, so that the DisplayPort driver can enable and disable the lane as needed. The DisplayPort mode is selected according to lane<n>-mode -property. All other values of lane<n>-mode -property but PHY_TYPE_DP will set P_STANDARD_MODE bits to 0 and P_ENABLE bits to force enable. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/phy/ti/phy-j721e-wiz.c | 55 +++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c index c74979655654..d2a38ea6d881 100644 --- a/drivers/phy/ti/phy-j721e-wiz.c +++ b/drivers/phy/ti/phy-j721e-wiz.c @@ -20,6 +20,7 @@ #include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset-controller.h> +#include <dt-bindings/phy/phy.h> #define WIZ_SERDES_CTRL 0x404 #define WIZ_SERDES_TOP_CTRL 0x408 @@ -78,6 +79,8 @@ static const struct reg_field p_enable[WIZ_MAX_LANES] = { REG_FIELD(WIZ_LANECTL(3), 30, 31), }; +enum p_enable { P_ENABLE = 2, P_ENABLE_FORCE = 1, P_ENABLE_DISABLE = 0 }; + static const struct reg_field p_align[WIZ_MAX_LANES] = { REG_FIELD(WIZ_LANECTL(0), 29, 29), REG_FIELD(WIZ_LANECTL(1), 29, 29), @@ -220,6 +223,7 @@ struct wiz { struct reset_controller_dev wiz_phy_reset_dev; struct gpio_desc *gpio_typec_dir; int typec_dir_delay; + u32 lane_modes[WIZ_MAX_LANES]; }; static int wiz_reset(struct wiz *wiz) @@ -242,12 +246,17 @@ static int wiz_reset(struct wiz *wiz) static int wiz_mode_select(struct wiz *wiz) { u32 num_lanes = wiz->num_lanes; + enum wiz_lane_standard_mode mode; int ret; int i; for (i = 0; i < num_lanes; i++) { - ret = regmap_field_write(wiz->p_standard_mode[i], - LANE_MODE_GEN4); + if (wiz->lane_modes[i] == PHY_TYPE_DP) + mode = LANE_MODE_GEN1; + else + mode = LANE_MODE_GEN4; + + ret = regmap_field_write(wiz->p_standard_mode[i], mode); if (ret) return ret; } @@ -713,7 +722,7 @@ static int wiz_phy_reset_assert(struct reset_controller_dev *rcdev, return ret; } - ret = regmap_field_write(wiz->p_enable[id - 1], false); + ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_DISABLE); return ret; } @@ -740,7 +749,11 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev, return ret; } - ret = regmap_field_write(wiz->p_enable[id - 1], true); + if (wiz->lane_modes[id - 1] == PHY_TYPE_DP) + ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE); + else + ret = regmap_field_write(wiz->p_enable[id - 1], P_ENABLE_FORCE); + return ret; } @@ -767,6 +780,33 @@ static const struct of_device_id wiz_id_table[] = { }; MODULE_DEVICE_TABLE(of, wiz_id_table); +static int wiz_get_lane_mode(struct device *dev, int lane_number, + u32 *lane_mode) +{ + char property_name[11]; /* 11 is length of "lane0-mode\0" */ + int ret; + + ret = snprintf(property_name, sizeof(property_name), "lane%u-mode", + lane_number); + + if (ret != 10) { /* 10 is length of "lane0-mode" */ + dev_err(dev, "%s: bad lane number %d (ret = %d)\n", + __func__, lane_number, ret); + return -ENOTSUPP; + } + + ret = of_property_read_u32(dev->of_node, property_name, lane_mode); + if (ret == -EINVAL) { + *lane_mode = PHY_NONE; + return 0; + } else if (ret) { + dev_err(dev, "Getting \"%s\" property failed: %d\n", + property_name, ret); + } + + return ret; +} + static int wiz_probe(struct platform_device *pdev) { struct reset_controller_dev *phy_reset_dev; @@ -780,6 +820,7 @@ static int wiz_probe(struct platform_device *pdev) struct wiz *wiz; u32 num_lanes; int ret; + int i; wiz = devm_kzalloc(dev, sizeof(*wiz), GFP_KERNEL); if (!wiz) @@ -850,6 +891,12 @@ static int wiz_probe(struct platform_device *pdev) } } + for (i = 0; i < num_lanes; i++) { + ret = wiz_get_lane_mode(dev, i, &wiz->lane_modes[i]); + if (ret) + return ret; + } + wiz->dev = dev; wiz->regmap = regmap; wiz->num_lanes = num_lanes; -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-12-30 18:22 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-09 16:20 [PATCH 0/3] phy: ti: j721e-wiz: Add support for DisplayPort mode Jyri Sarha 2019-12-09 16:21 ` [PATCH 1/3] dt-bindings: phy: Add PHY_TYPE_DP definition Jyri Sarha 2019-12-19 19:04 ` Rob Herring 2019-12-09 16:22 ` [PATCH 2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper) Jyri Sarha 2019-12-19 19:08 ` Rob Herring 2019-12-20 12:52 ` Jyri Sarha 2019-12-24 21:31 ` Rob Herring 2019-12-30 9:37 ` Jyri Sarha 2019-12-30 10:18 ` Kishon Vijay Abraham I 2019-12-30 11:54 ` Jyri Sarha 2019-12-30 18:21 ` Rob Herring 2019-12-09 16:22 ` [PATCH 3/3] phy: ti: j721e-wiz: Implement DisplayPort mode to the wiz driver Jyri Sarha
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).