mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors
@ 2026-07-23  2:38 Pengpeng Hou
  2026-07-25  0:55 ` Doug Anderson
  2026-07-27 11:40 ` Jani Nikula
  0 siblings, 2 replies; 6+ messages in thread
From: Pengpeng Hou @ 2026-07-23  2:38 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel, Pengpeng Hou

ps8640_aux_transfer_msg() programs the AUX address registers, starts the
AUX transfer, waits for SWAUX_SEND to clear, and reads the AUX status
register. Several of those regmap operations have return values, but the
function only checks a stale ret after the status read.

Propagate failures from the address write, transfer start, completion
poll, and status read. This avoids returning a transfer length when the
bridge register transaction or AUX completion wait failed.

Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX channel")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1: https://lore.kernel.org/all/20260623060635.16064-1-pengpeng@iscas.ac.cn/
- include the AUX address and length in the address-write diagnostic
- describe poll failures without assuming that every error is a timeout
- add the Fixes tag suggested by Doug
- rebase onto the current drm-misc-next tree

 drivers/gpu/drm/bridge/parade-ps8640.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
index 96332721cb69..56b48f8feab6 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -257,8 +257,14 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
 	addr_len[PAGE0_SWAUX_LENGTH - base] = (len == 0) ? SWAUX_NO_PAYLOAD :
 					      ((len - 1) & SWAUX_LENGTH_MASK);
 
-	regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
-			  ARRAY_SIZE(addr_len));
+	ret = regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
+				ARRAY_SIZE(addr_len));
+	if (ret) {
+		DRM_DEV_ERROR(dev,
+			      "failed to write AUX address %#x, len %zu: %d\n",
+			      msg->address, len, ret);
+		return ret;
+	}
 
 	if (len && (request == DP_AUX_NATIVE_WRITE ||
 		    request == DP_AUX_I2C_WRITE)) {
@@ -274,13 +280,22 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
 		}
 	}
 
-	regmap_write(map, PAGE0_SWAUX_CTRL, SWAUX_SEND);
+	ret = regmap_write(map, PAGE0_SWAUX_CTRL, SWAUX_SEND);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to start AUX transfer: %d\n", ret);
+		return ret;
+	}
 
 	/* Zero delay loop because i2c transactions are slow already */
-	regmap_read_poll_timeout(map, PAGE0_SWAUX_CTRL, data,
-				 !(data & SWAUX_SEND), 0, 50 * 1000);
+	ret = regmap_read_poll_timeout(map, PAGE0_SWAUX_CTRL, data,
+				       !(data & SWAUX_SEND), 0, 50 * 1000);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to complete AUX transfer: %d\n",
+			      ret);
+		return ret;
+	}
 
-	regmap_read(map, PAGE0_SWAUX_STATUS, &data);
+	ret = regmap_read(map, PAGE0_SWAUX_STATUS, &data);
 	if (ret) {
 		DRM_DEV_ERROR(dev, "failed to read PAGE0_SWAUX_STATUS: %d\n",
 			      ret);
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors
  2026-07-23  2:38 [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors Pengpeng Hou
@ 2026-07-25  0:55 ` Doug Anderson
  2026-07-27 11:40 ` Jani Nikula
  1 sibling, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2026-07-25  0:55 UTC (permalink / raw)
  To: Pengpeng Hou
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel

Hi,

On Wed, Jul 22, 2026 at 7:38 PM Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
>
> ps8640_aux_transfer_msg() programs the AUX address registers, starts the
> AUX transfer, waits for SWAUX_SEND to clear, and reads the AUX status
> register. Several of those regmap operations have return values, but the
> function only checks a stale ret after the status read.
>
> Propagate failures from the address write, transfer start, completion
> poll, and status read. This avoids returning a transfer length when the
> bridge register transaction or AUX completion wait failed.
>
> Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX channel")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1: https://lore.kernel.org/all/20260623060635.16064-1-pengpeng@iscas.ac.cn/
> - include the AUX address and length in the address-write diagnostic
> - describe poll failures without assuming that every error is a timeout
> - add the Fixes tag suggested by Doug
> - rebase onto the current drm-misc-next tree
>
>  drivers/gpu/drm/bridge/parade-ps8640.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Assuming nothing else comes up, I'll plan to apply to drm-misc-fixes next week.

-Doug

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors
  2026-07-23  2:38 [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors Pengpeng Hou
  2026-07-25  0:55 ` Doug Anderson
@ 2026-07-27 11:40 ` Jani Nikula
  2026-07-30  1:41   ` Doug Anderson
  1 sibling, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2026-07-27 11:40 UTC (permalink / raw)
  To: Pengpeng Hou, Douglas Anderson
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel, Pengpeng Hou

On Thu, 23 Jul 2026, Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
> ps8640_aux_transfer_msg() programs the AUX address registers, starts the
> AUX transfer, waits for SWAUX_SEND to clear, and reads the AUX status
> register. Several of those regmap operations have return values, but the
> function only checks a stale ret after the status read.
>
> Propagate failures from the address write, transfer start, completion
> poll, and status read. This avoids returning a transfer length when the
> bridge register transaction or AUX completion wait failed.
>
> Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX channel")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1: https://lore.kernel.org/all/20260623060635.16064-1-pengpeng@iscas.ac.cn/
> - include the AUX address and length in the address-write diagnostic
> - describe poll failures without assuming that every error is a timeout
> - add the Fixes tag suggested by Doug
> - rebase onto the current drm-misc-next tree
>
>  drivers/gpu/drm/bridge/parade-ps8640.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 96332721cb69..56b48f8feab6 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -257,8 +257,14 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
>  	addr_len[PAGE0_SWAUX_LENGTH - base] = (len == 0) ? SWAUX_NO_PAYLOAD :
>  					      ((len - 1) & SWAUX_LENGTH_MASK);
>  
> -	regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> -			  ARRAY_SIZE(addr_len));
> +	ret = regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> +				ARRAY_SIZE(addr_len));
> +	if (ret) {
> +		DRM_DEV_ERROR(dev,

DRM_DEV_ERROR() is deprecated in favour of drm_err() or dev_err().

BR,
Jani.

> +			      "failed to write AUX address %#x, len %zu: %d\n",
> +			      msg->address, len, ret);
> +		return ret;
> +	}
>  
>  	if (len && (request == DP_AUX_NATIVE_WRITE ||
>  		    request == DP_AUX_I2C_WRITE)) {
> @@ -274,13 +280,22 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
>  		}
>  	}
>  
> -	regmap_write(map, PAGE0_SWAUX_CTRL, SWAUX_SEND);
> +	ret = regmap_write(map, PAGE0_SWAUX_CTRL, SWAUX_SEND);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to start AUX transfer: %d\n", ret);
> +		return ret;
> +	}
>  
>  	/* Zero delay loop because i2c transactions are slow already */
> -	regmap_read_poll_timeout(map, PAGE0_SWAUX_CTRL, data,
> -				 !(data & SWAUX_SEND), 0, 50 * 1000);
> +	ret = regmap_read_poll_timeout(map, PAGE0_SWAUX_CTRL, data,
> +				       !(data & SWAUX_SEND), 0, 50 * 1000);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to complete AUX transfer: %d\n",
> +			      ret);
> +		return ret;
> +	}
>  
> -	regmap_read(map, PAGE0_SWAUX_STATUS, &data);
> +	ret = regmap_read(map, PAGE0_SWAUX_STATUS, &data);
>  	if (ret) {
>  		DRM_DEV_ERROR(dev, "failed to read PAGE0_SWAUX_STATUS: %d\n",
>  			      ret);

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors
  2026-07-27 11:40 ` Jani Nikula
@ 2026-07-30  1:41   ` Doug Anderson
  2026-07-30  8:38     ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2026-07-30  1:41 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Pengpeng Hou, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, dri-devel, linux-kernel

Hi,

On Mon, Jul 27, 2026 at 4:40 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Thu, 23 Jul 2026, Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
> > ps8640_aux_transfer_msg() programs the AUX address registers, starts the
> > AUX transfer, waits for SWAUX_SEND to clear, and reads the AUX status
> > register. Several of those regmap operations have return values, but the
> > function only checks a stale ret after the status read.
> >
> > Propagate failures from the address write, transfer start, completion
> > poll, and status read. This avoids returning a transfer length when the
> > bridge register transaction or AUX completion wait failed.
> >
> > Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX channel")
> > Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> > ---
> > Changes since v1: https://lore.kernel.org/all/20260623060635.16064-1-pengpeng@iscas.ac.cn/
> > - include the AUX address and length in the address-write diagnostic
> > - describe poll failures without assuming that every error is a timeout
> > - add the Fixes tag suggested by Doug
> > - rebase onto the current drm-misc-next tree
> >
> >  drivers/gpu/drm/bridge/parade-ps8640.c | 27 +++++++++++++++++++++------
> >  1 file changed, 21 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> > index 96332721cb69..56b48f8feab6 100644
> > --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> > @@ -257,8 +257,14 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
> >       addr_len[PAGE0_SWAUX_LENGTH - base] = (len == 0) ? SWAUX_NO_PAYLOAD :
> >                                             ((len - 1) & SWAUX_LENGTH_MASK);
> >
> > -     regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> > -                       ARRAY_SIZE(addr_len));
> > +     ret = regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> > +                             ARRAY_SIZE(addr_len));
> > +     if (ret) {
> > +             DRM_DEV_ERROR(dev,
>
> DRM_DEV_ERROR() is deprecated in favour of drm_err() or dev_err().

Good point. I'm inclined to land this anyway, though, since this is
nominally a bugfix and it matches the error printing in the rest of
the function. Any objections? It would be a nice future cleanup to fix
the error printing in this driver overall to use the non-deprecated
functions...

-Doug

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors
  2026-07-30  1:41   ` Doug Anderson
@ 2026-07-30  8:38     ` Jani Nikula
  2026-07-30 21:26       ` Doug Anderson
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2026-07-30  8:38 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Pengpeng Hou, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, dri-devel, linux-kernel

On Wed, 29 Jul 2026, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Mon, Jul 27, 2026 at 4:40 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>
>> On Thu, 23 Jul 2026, Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
>> > ps8640_aux_transfer_msg() programs the AUX address registers, starts the
>> > AUX transfer, waits for SWAUX_SEND to clear, and reads the AUX status
>> > register. Several of those regmap operations have return values, but the
>> > function only checks a stale ret after the status read.
>> >
>> > Propagate failures from the address write, transfer start, completion
>> > poll, and status read. This avoids returning a transfer length when the
>> > bridge register transaction or AUX completion wait failed.
>> >
>> > Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX channel")
>> > Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
>> > ---
>> > Changes since v1: https://lore.kernel.org/all/20260623060635.16064-1-pengpeng@iscas.ac.cn/
>> > - include the AUX address and length in the address-write diagnostic
>> > - describe poll failures without assuming that every error is a timeout
>> > - add the Fixes tag suggested by Doug
>> > - rebase onto the current drm-misc-next tree
>> >
>> >  drivers/gpu/drm/bridge/parade-ps8640.c | 27 +++++++++++++++++++++------
>> >  1 file changed, 21 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
>> > index 96332721cb69..56b48f8feab6 100644
>> > --- a/drivers/gpu/drm/bridge/parade-ps8640.c
>> > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
>> > @@ -257,8 +257,14 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
>> >       addr_len[PAGE0_SWAUX_LENGTH - base] = (len == 0) ? SWAUX_NO_PAYLOAD :
>> >                                             ((len - 1) & SWAUX_LENGTH_MASK);
>> >
>> > -     regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
>> > -                       ARRAY_SIZE(addr_len));
>> > +     ret = regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
>> > +                             ARRAY_SIZE(addr_len));
>> > +     if (ret) {
>> > +             DRM_DEV_ERROR(dev,
>>
>> DRM_DEV_ERROR() is deprecated in favour of drm_err() or dev_err().
>
> Good point. I'm inclined to land this anyway, though, since this is
> nominally a bugfix and it matches the error printing in the rest of
> the function. Any objections? It would be a nice future cleanup to fix
> the error printing in this driver overall to use the non-deprecated
> functions...

Up to you.

BR,
Jani.


-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors
  2026-07-30  8:38     ` Jani Nikula
@ 2026-07-30 21:26       ` Doug Anderson
  0 siblings, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2026-07-30 21:26 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Pengpeng Hou, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, dri-devel, linux-kernel

Hi,

On Thu, Jul 30, 2026 at 1:38 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Wed, 29 Jul 2026, Doug Anderson <dianders@chromium.org> wrote:
> > Hi,
> >
> > On Mon, Jul 27, 2026 at 4:40 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >>
> >> On Thu, 23 Jul 2026, Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
> >> > ps8640_aux_transfer_msg() programs the AUX address registers, starts the
> >> > AUX transfer, waits for SWAUX_SEND to clear, and reads the AUX status
> >> > register. Several of those regmap operations have return values, but the
> >> > function only checks a stale ret after the status read.
> >> >
> >> > Propagate failures from the address write, transfer start, completion
> >> > poll, and status read. This avoids returning a transfer length when the
> >> > bridge register transaction or AUX completion wait failed.
> >> >
> >> > Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX channel")
> >> > Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> >> > ---
> >> > Changes since v1: https://lore.kernel.org/all/20260623060635.16064-1-pengpeng@iscas.ac.cn/
> >> > - include the AUX address and length in the address-write diagnostic
> >> > - describe poll failures without assuming that every error is a timeout
> >> > - add the Fixes tag suggested by Doug
> >> > - rebase onto the current drm-misc-next tree
> >> >
> >> >  drivers/gpu/drm/bridge/parade-ps8640.c | 27 +++++++++++++++++++++------
> >> >  1 file changed, 21 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> >> > index 96332721cb69..56b48f8feab6 100644
> >> > --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> >> > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> >> > @@ -257,8 +257,14 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
> >> >       addr_len[PAGE0_SWAUX_LENGTH - base] = (len == 0) ? SWAUX_NO_PAYLOAD :
> >> >                                             ((len - 1) & SWAUX_LENGTH_MASK);
> >> >
> >> > -     regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> >> > -                       ARRAY_SIZE(addr_len));
> >> > +     ret = regmap_bulk_write(map, PAGE0_SWAUX_ADDR_7_0, addr_len,
> >> > +                             ARRAY_SIZE(addr_len));
> >> > +     if (ret) {
> >> > +             DRM_DEV_ERROR(dev,
> >>
> >> DRM_DEV_ERROR() is deprecated in favour of drm_err() or dev_err().
> >
> > Good point. I'm inclined to land this anyway, though, since this is
> > nominally a bugfix and it matches the error printing in the rest of
> > the function. Any objections? It would be a nice future cleanup to fix
> > the error printing in this driver overall to use the non-deprecated
> > functions...
>
> Up to you.

OK, pushed to drm-misc-fixes.

[1/1] drm/bridge: ps8640: propagate AUX transfer register errors
      commit: 20697ecb299cd77b4cf8b28f655e56606b0472d8

It would be great if someone wanted to update the error prints. :-)

-Doug

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-30 21:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-23  2:38 [PATCH v2] drm/bridge: ps8640: propagate AUX transfer register errors Pengpeng Hou
2026-07-25  0:55 ` Doug Anderson
2026-07-27 11:40 ` Jani Nikula
2026-07-30  1:41   ` Doug Anderson
2026-07-30  8:38     ` Jani Nikula
2026-07-30 21:26       ` Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome