| Message ID | 6109476a-e8fa-6d82-3ed8-3833f0f18615@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i2c/tda9950.c: set MAX_RETRIES for errors only | expand |
Russell (or someone else), can you Ack this patch? I'd like to get this for 4.20. Thanks! Hans On 08/27/2018 02:28 PM, Hans Verkuil wrote: > The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to > prevent the CEC framework from retrying the transmit. If the > transmit was successful, then don't set this flag. > > Found by running 'cec-compliance -A' on a beaglebone box. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/gpu/drm/i2c/tda9950.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > index 5d2f0d548469..4a14fc3b5011 100644 > --- a/drivers/gpu/drm/i2c/tda9950.c > +++ b/drivers/gpu/drm/i2c/tda9950.c > @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) > break; > } > /* TDA9950 executes all retries for us */ > - tx_status |= CEC_TX_STATUS_MAX_RETRIES; > + if (tx_status != CEC_TX_STATUS_OK) > + tx_status |= CEC_TX_STATUS_MAX_RETRIES; > cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, > nack_cnt, 0, err_cnt); > break; >
Hi Hans, Thanks for the patch. On Mon, Aug 27, 2018 at 02:28:50PM +0200, Hans Verkuil wrote: > The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to > prevent the CEC framework from retrying the transmit. If the > transmit was successful, then don't set this flag. > > Found by running 'cec-compliance -A' on a beaglebone box. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/gpu/drm/i2c/tda9950.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > index 5d2f0d548469..4a14fc3b5011 100644 > --- a/drivers/gpu/drm/i2c/tda9950.c > +++ b/drivers/gpu/drm/i2c/tda9950.c > @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) > break; > } > /* TDA9950 executes all retries for us */ > - tx_status |= CEC_TX_STATUS_MAX_RETRIES; > + if (tx_status != CEC_TX_STATUS_OK) > + tx_status |= CEC_TX_STATUS_MAX_RETRIES; > cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, > nack_cnt, 0, err_cnt); > break; Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.com> > -- > 2.18.0 > >
Hi Hans, I'll pick it up in due course. Thanks. On Tue, Sep 11, 2018 at 08:41:59AM +0200, Hans Verkuil wrote: > Russell (or someone else), can you Ack this patch? I'd like to get this > for 4.20. > > Thanks! > > Hans > > On 08/27/2018 02:28 PM, Hans Verkuil wrote: > > The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to > > prevent the CEC framework from retrying the transmit. If the > > transmit was successful, then don't set this flag. > > > > Found by running 'cec-compliance -A' on a beaglebone box. > > > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > > --- > > drivers/gpu/drm/i2c/tda9950.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > > index 5d2f0d548469..4a14fc3b5011 100644 > > --- a/drivers/gpu/drm/i2c/tda9950.c > > +++ b/drivers/gpu/drm/i2c/tda9950.c > > @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) > > break; > > } > > /* TDA9950 executes all retries for us */ > > - tx_status |= CEC_TX_STATUS_MAX_RETRIES; > > + if (tx_status != CEC_TX_STATUS_OK) > > + tx_status |= CEC_TX_STATUS_MAX_RETRIES; > > cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, > > nack_cnt, 0, err_cnt); > > break; > > >
On Thu, Sep 13, 2018 at 10:33:35AM +0100, Russell King - ARM Linux wrote: > Hi Hans, > > I'll pick it up in due course. > > Thanks. > > On Tue, Sep 11, 2018 at 08:41:59AM +0200, Hans Verkuil wrote: > > Russell (or someone else), can you Ack this patch? I'd like to get this > > for 4.20. > > > > Thanks! > > > > Hans > > > > On 08/27/2018 02:28 PM, Hans Verkuil wrote: > > > The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to > > > prevent the CEC framework from retrying the transmit. If the > > > transmit was successful, then don't set this flag. > > > > > > Found by running 'cec-compliance -A' on a beaglebone box. > > > > > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> Since the tda driver is now a brideg one, would make sense to maintain it as part of drm-misc? Hans could push directly then. -Daniel > > > --- > > > drivers/gpu/drm/i2c/tda9950.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > > > index 5d2f0d548469..4a14fc3b5011 100644 > > > --- a/drivers/gpu/drm/i2c/tda9950.c > > > +++ b/drivers/gpu/drm/i2c/tda9950.c > > > @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) > > > break; > > > } > > > /* TDA9950 executes all retries for us */ > > > - tx_status |= CEC_TX_STATUS_MAX_RETRIES; > > > + if (tx_status != CEC_TX_STATUS_OK) > > > + tx_status |= CEC_TX_STATUS_MAX_RETRIES; > > > cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, > > > nack_cnt, 0, err_cnt); > > > break; > > > > > > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up > According to speedtest.net: 13Mbps down 490kbps up > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 09/13/18 15:16, Daniel Vetter wrote: > On Thu, Sep 13, 2018 at 10:33:35AM +0100, Russell King - ARM Linux wrote: >> Hi Hans, >> >> I'll pick it up in due course. >> >> Thanks. >> >> On Tue, Sep 11, 2018 at 08:41:59AM +0200, Hans Verkuil wrote: >>> Russell (or someone else), can you Ack this patch? I'd like to get this >>> for 4.20. >>> >>> Thanks! >>> >>> Hans >>> >>> On 08/27/2018 02:28 PM, Hans Verkuil wrote: >>>> The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to >>>> prevent the CEC framework from retrying the transmit. If the >>>> transmit was successful, then don't set this flag. >>>> >>>> Found by running 'cec-compliance -A' on a beaglebone box. >>>> >>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > > Since the tda driver is now a brideg one, would make sense to maintain it > as part of drm-misc? Hans could push directly then. It isn't yet part of drm-misc? It would make sense IMHO. And 'due course' is too vague since this should be merged for 4.20. I plan to add BeagleBone Black support soon for 4.20 since the GPIO issues that blocked supporting that board are close to being resolved. And this should be fixed before enabling BBB support. It's an annoying bug that trips up the cec-compliance adapter test. Regards, Hans > -Daniel > >>>> --- >>>> drivers/gpu/drm/i2c/tda9950.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c >>>> index 5d2f0d548469..4a14fc3b5011 100644 >>>> --- a/drivers/gpu/drm/i2c/tda9950.c >>>> +++ b/drivers/gpu/drm/i2c/tda9950.c >>>> @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) >>>> break; >>>> } >>>> /* TDA9950 executes all retries for us */ >>>> - tx_status |= CEC_TX_STATUS_MAX_RETRIES; >>>> + if (tx_status != CEC_TX_STATUS_OK) >>>> + tx_status |= CEC_TX_STATUS_MAX_RETRIES; >>>> cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, >>>> nack_cnt, 0, err_cnt); >>>> break; >>>> >>> >> >> -- >> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ >> FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up >> According to speedtest.net: 13Mbps down 490kbps up >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Thu, Sep 13, 2018 at 03:33:20PM +0200, Hans Verkuil wrote: > On 09/13/18 15:16, Daniel Vetter wrote: > > On Thu, Sep 13, 2018 at 10:33:35AM +0100, Russell King - ARM Linux wrote: > >> Hi Hans, > >> > >> I'll pick it up in due course. > >> > >> Thanks. > >> > >> On Tue, Sep 11, 2018 at 08:41:59AM +0200, Hans Verkuil wrote: > >>> Russell (or someone else), can you Ack this patch? I'd like to get this > >>> for 4.20. > >>> > >>> Thanks! > >>> > >>> Hans > >>> > >>> On 08/27/2018 02:28 PM, Hans Verkuil wrote: > >>>> The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to > >>>> prevent the CEC framework from retrying the transmit. If the > >>>> transmit was successful, then don't set this flag. > >>>> > >>>> Found by running 'cec-compliance -A' on a beaglebone box. > >>>> > >>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > > > > Since the tda driver is now a brideg one, would make sense to maintain it > > as part of drm-misc? Hans could push directly then. > > It isn't yet part of drm-misc? It would make sense IMHO. I think not formally, drm/i2c isn't one of the drm-misc areas. But it does fall under "everything else" exception. And I'd be happy to ack a formal MAINTAINERS patch (plus maybe even moving it from drm/i2c/ to drm/bridge/). > And 'due course' is too vague since this should be merged for 4.20. > I plan to add BeagleBone Black support soon for 4.20 since the GPIO issues > that blocked supporting that board are close to being resolved. And this > should be fixed before enabling BBB support. > > It's an annoying bug that trips up the cec-compliance adapter test. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> for stuffing right away into drm-misc-next, if that helps. -Daniel > > Regards, > > Hans > > > -Daniel > > > >>>> --- > >>>> drivers/gpu/drm/i2c/tda9950.c | 3 ++- > >>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > >>>> index 5d2f0d548469..4a14fc3b5011 100644 > >>>> --- a/drivers/gpu/drm/i2c/tda9950.c > >>>> +++ b/drivers/gpu/drm/i2c/tda9950.c > >>>> @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) > >>>> break; > >>>> } > >>>> /* TDA9950 executes all retries for us */ > >>>> - tx_status |= CEC_TX_STATUS_MAX_RETRIES; > >>>> + if (tx_status != CEC_TX_STATUS_OK) > >>>> + tx_status |= CEC_TX_STATUS_MAX_RETRIES; > >>>> cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, > >>>> nack_cnt, 0, err_cnt); > >>>> break; > >>>> > >>> > >> > >> -- > >> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > >> FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up > >> According to speedtest.net: 13Mbps down 490kbps up > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > >
On Thu, Sep 13, 2018 at 03:33:20PM +0200, Hans Verkuil wrote: > On 09/13/18 15:16, Daniel Vetter wrote: > > On Thu, Sep 13, 2018 at 10:33:35AM +0100, Russell King - ARM Linux wrote: > >> Hi Hans, > >> > >> I'll pick it up in due course. > >> > >> Thanks. > >> > >> On Tue, Sep 11, 2018 at 08:41:59AM +0200, Hans Verkuil wrote: > >>> Russell (or someone else), can you Ack this patch? I'd like to get this > >>> for 4.20. > >>> > >>> Thanks! > >>> > >>> Hans > >>> > >>> On 08/27/2018 02:28 PM, Hans Verkuil wrote: > >>>> The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to > >>>> prevent the CEC framework from retrying the transmit. If the > >>>> transmit was successful, then don't set this flag. > >>>> > >>>> Found by running 'cec-compliance -A' on a beaglebone box. > >>>> > >>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > > > > Since the tda driver is now a brideg one, would make sense to maintain it > > as part of drm-misc? Hans could push directly then. > > It isn't yet part of drm-misc? It would make sense IMHO. > > And 'due course' is too vague since this should be merged for 4.20. Given that we are at 4.19-rc3, and you are talking about it being merged during the _next_ merge window, there is plenty of time remaining that waiting another week or two for me to pick it up is not a problem. In any case, my plan is to merge it for 4.19 since it appears to be a bug fix, albiet a minor one.
On 09/13/18 15:48, Russell King - ARM Linux wrote: > On Thu, Sep 13, 2018 at 03:33:20PM +0200, Hans Verkuil wrote: >> On 09/13/18 15:16, Daniel Vetter wrote: >>> On Thu, Sep 13, 2018 at 10:33:35AM +0100, Russell King - ARM Linux wrote: >>>> Hi Hans, >>>> >>>> I'll pick it up in due course. >>>> >>>> Thanks. >>>> >>>> On Tue, Sep 11, 2018 at 08:41:59AM +0200, Hans Verkuil wrote: >>>>> Russell (or someone else), can you Ack this patch? I'd like to get this >>>>> for 4.20. >>>>> >>>>> Thanks! >>>>> >>>>> Hans >>>>> >>>>> On 08/27/2018 02:28 PM, Hans Verkuil wrote: >>>>>> The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to >>>>>> prevent the CEC framework from retrying the transmit. If the >>>>>> transmit was successful, then don't set this flag. >>>>>> >>>>>> Found by running 'cec-compliance -A' on a beaglebone box. >>>>>> >>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> >>> >>> Since the tda driver is now a brideg one, would make sense to maintain it >>> as part of drm-misc? Hans could push directly then. >> >> It isn't yet part of drm-misc? It would make sense IMHO. >> >> And 'due course' is too vague since this should be merged for 4.20. > > Given that we are at 4.19-rc3, and you are talking about it being merged > during the _next_ merge window, there is plenty of time remaining that > waiting another week or two for me to pick it up is not a problem. No problem, then I leave it to you to pick up. 'due course' can mean anything from tomorrow to next year, so that didn't help me :-) > In any case, my plan is to merge it for 4.19 since it appears to be a > bug fix, albiet a minor one. > It's a bug fix, but nothing in the kernel tree is currently using this AFAIK. The BBB would be the first to actually activate it. I'm fine with merging it in 4.19, but it is not strictly necessary. Regards, Hans
Hi Russell, On Thu, Sep 13, 2018 at 3:48 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Thu, Sep 13, 2018 at 03:33:20PM +0200, Hans Verkuil wrote: >> On 09/13/18 15:16, Daniel Vetter wrote: >> > On Thu, Sep 13, 2018 at 10:33:35AM +0100, Russell King - ARM Linux wrote: >> >> Hi Hans, >> >> >> >> I'll pick it up in due course. >> >> >> >> Thanks. >> >> >> >> On Tue, Sep 11, 2018 at 08:41:59AM +0200, Hans Verkuil wrote: >> >>> Russell (or someone else), can you Ack this patch? I'd like to get this >> >>> for 4.20. >> >>> >> >>> Thanks! >> >>> >> >>> Hans >> >>> >> >>> On 08/27/2018 02:28 PM, Hans Verkuil wrote: >> >>>> The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to >> >>>> prevent the CEC framework from retrying the transmit. If the >> >>>> transmit was successful, then don't set this flag. >> >>>> >> >>>> Found by running 'cec-compliance -A' on a beaglebone box. >> >>>> >> >>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> >> > >> > Since the tda driver is now a brideg one, would make sense to maintain it >> > as part of drm-misc? Hans could push directly then. >> >> It isn't yet part of drm-misc? It would make sense IMHO. >> >> And 'due course' is too vague since this should be merged for 4.20. > > Given that we are at 4.19-rc3, and you are talking about it being merged > during the _next_ merge window, there is plenty of time remaining that > waiting another week or two for me to pick it up is not a problem. > > In any case, my plan is to merge it for 4.19 since it appears to be a > bug fix, albiet a minor one. Thanks for handling tdaxxxx.c in an efficient manner. And to clarify: drm-misc is just an option that's out there, and you're obviously very much welcome to join (commit rights included ofc). I do personally think it's great to have an informal group maintainership like in drm-misc where people can easily jump in&out of helping out with specific drivers, but it's by no means mandatory. There's lots of options to effectively and efficiently collaborate. Thanks, Daniel
Hi Hans, Patch merged, thanks. On Mon, Aug 27, 2018 at 02:28:50PM +0200, Hans Verkuil wrote: > The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to > prevent the CEC framework from retrying the transmit. If the > transmit was successful, then don't set this flag. > > Found by running 'cec-compliance -A' on a beaglebone box. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/gpu/drm/i2c/tda9950.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > index 5d2f0d548469..4a14fc3b5011 100644 > --- a/drivers/gpu/drm/i2c/tda9950.c > +++ b/drivers/gpu/drm/i2c/tda9950.c > @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) > break; > } > /* TDA9950 executes all retries for us */ > - tx_status |= CEC_TX_STATUS_MAX_RETRIES; > + if (tx_status != CEC_TX_STATUS_OK) > + tx_status |= CEC_TX_STATUS_MAX_RETRIES; > cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, > nack_cnt, 0, err_cnt); > break; > -- > 2.18.0 > >
diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c index 5d2f0d548469..4a14fc3b5011 100644 --- a/drivers/gpu/drm/i2c/tda9950.c +++ b/drivers/gpu/drm/i2c/tda9950.c @@ -191,7 +191,8 @@ static irqreturn_t tda9950_irq(int irq, void *data) break; } /* TDA9950 executes all retries for us */ - tx_status |= CEC_TX_STATUS_MAX_RETRIES; + if (tx_status != CEC_TX_STATUS_OK) + tx_status |= CEC_TX_STATUS_MAX_RETRIES; cec_transmit_done(priv->adap, tx_status, arb_lost_cnt, nack_cnt, 0, err_cnt); break;
The CEC_TX_STATUS_MAX_RETRIES should be set for errors only to prevent the CEC framework from retrying the transmit. If the transmit was successful, then don't set this flag. Found by running 'cec-compliance -A' on a beaglebone box. Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> --- drivers/gpu/drm/i2c/tda9950.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)