| Message ID | 20240502-anx-tdm-v1-1-894a9f634f44@chromium.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add TDM setting support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote: > Add a perporty to indicate whether anx7625 should shift the first audio > data bit. The default TDM setting is to shift the first audio data bit. > > Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org> > --- > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > index a1ed1004651b9..915d5d54a2160 100644 > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > @@ -82,6 +82,10 @@ properties: > type: boolean > description: let the driver enable audio HDMI codec function or not. > > + no-shift-audio-data: > + type: boolean > + description: Disable the first audio data bit shift in the TDM settings. This just looks like software policy, since there's no mention in the commit message or description as to what property of the hardware causes this to be required. Can you please explain why this property is needed? You're also missing a vendor prefix. Thanks, Conor. > + > aux-bus: > $ref: /schemas/display/dp-aux-bus.yaml# > > > -- > 2.45.0.rc1.225.g2a3ae87e7f-goog >
On Thu, May 2, 2024 at 10:47 PM Conor Dooley <conor@kernel.org> wrote: > > On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote: > > Add a perporty to indicate whether anx7625 should shift the first audio > > data bit. The default TDM setting is to shift the first audio data bit. > > > > Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org> > > --- > > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > index a1ed1004651b9..915d5d54a2160 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > @@ -82,6 +82,10 @@ properties: > > type: boolean > > description: let the driver enable audio HDMI codec function or not. > > > > + no-shift-audio-data: > > + type: boolean > > + description: Disable the first audio data bit shift in the TDM settings. > > This just looks like software policy, since there's no mention in the > commit message or description as to what property of the hardware causes > this to be required. Can you please explain why this property is needed? > > You're also missing a vendor prefix. > > Thanks, > Conor. > > > + > > aux-bus: > > $ref: /schemas/display/dp-aux-bus.yaml# > > > > > > -- > > 2.45.0.rc1.225.g2a3ae87e7f-goog > > Sorry, I found this feature in the datasheet originally, but after deeper investigation, it seems that this feature should be used to support i2s dsp mode b instead of being used this way. Note that the difference between i2s dsp mode a and b is whether or not to shift the audio data by 1 clock cycle. Regards, Hsin-Te
On Fri, May 03, 2024 at 02:58:16PM +0800, Hsin-Te Yuan wrote: > On Thu, May 2, 2024 at 10:47 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote: > > > Add a perporty to indicate whether anx7625 should shift the first audio > > > data bit. The default TDM setting is to shift the first audio data bit. > > > > > > Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org> > > > --- > > > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > index a1ed1004651b9..915d5d54a2160 100644 > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > @@ -82,6 +82,10 @@ properties: > > > type: boolean > > > description: let the driver enable audio HDMI codec function or not. > > > > > > + no-shift-audio-data: > > > + type: boolean > > > + description: Disable the first audio data bit shift in the TDM settings. > > > > This just looks like software policy, since there's no mention in the > > commit message or description as to what property of the hardware causes > > this to be required. Can you please explain why this property is needed? > > > > You're also missing a vendor prefix. > > Sorry, I found this feature in the datasheet originally, but after > deeper investigation, it seems that this feature should be used to > support i2s dsp mode b instead of being used this way. Note that the > difference between i2s dsp mode a and b is whether or not to shift the > audio data by 1 clock cycle. Are you trying to say that this patch is not needed? I'm not really sure.
On Sat, May 4, 2024 at 12:45 AM Conor Dooley <conor@kernel.org> wrote: > > On Fri, May 03, 2024 at 02:58:16PM +0800, Hsin-Te Yuan wrote: > > On Thu, May 2, 2024 at 10:47 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Thu, May 02, 2024 at 09:03:31AM +0000, Hsin-Te Yuan wrote: > > > > Add a perporty to indicate whether anx7625 should shift the first audio > > > > data bit. The default TDM setting is to shift the first audio data bit. > > > > > > > > Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org> > > > > --- > > > > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > > index a1ed1004651b9..915d5d54a2160 100644 > > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > > @@ -82,6 +82,10 @@ properties: > > > > type: boolean > > > > description: let the driver enable audio HDMI codec function or not. > > > > > > > > + no-shift-audio-data: > > > > + type: boolean > > > > + description: Disable the first audio data bit shift in the TDM settings. > > > > > > This just looks like software policy, since there's no mention in the > > > commit message or description as to what property of the hardware causes > > > this to be required. Can you please explain why this property is needed? > > > > > > You're also missing a vendor prefix. > > > > Sorry, I found this feature in the datasheet originally, but after > > deeper investigation, it seems that this feature should be used to > > support i2s dsp mode b instead of being used this way. Note that the > > difference between i2s dsp mode a and b is whether or not to shift the > > audio data by 1 clock cycle. > > Are you trying to say that this patch is not needed? I'm not really > sure. Yes! Regards, Hsin-Te
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml index a1ed1004651b9..915d5d54a2160 100644 --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml @@ -82,6 +82,10 @@ properties: type: boolean description: let the driver enable audio HDMI codec function or not. + no-shift-audio-data: + type: boolean + description: Disable the first audio data bit shift in the TDM settings. + aux-bus: $ref: /schemas/display/dp-aux-bus.yaml#
Add a perporty to indicate whether anx7625 should shift the first audio data bit. The default TDM setting is to shift the first audio data bit. Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org> --- .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4 ++++ 1 file changed, 4 insertions(+)