Re: [PATCH 0/3] drm/bridge: ti-sn65dsi86: Some updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Laurent,

On Wed, Nov 29, 2023 at 02:45:33AM +0200, Laurent Pinchart wrote:
> On Fri, Nov 24, 2023 at 09:56:55AM +0100, Neil Armstrong wrote:
> > On 23/11/2023 18:54, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > this is a series I created while starring at the ti-sn65dsi86 driver in
> > > the context of my pwm-lifetime series.
> > > 
> > > The first patch should be fine. The last one has a few rough edges, but
> > > maybe you like the direction this is going to? The 2nd patch probably
> > > only makes sense if you also take the third.
> > > 
> > > Best regards
> > > Uwe
> > > 
> > > Uwe Kleine-König (3):
> > >    drm/bridge: ti-sn65dsi86: Simplify using pm_runtime_resume_and_get()
> > >    drm/bridge: ti-sn65dsi86: Change parameters of
> > >      ti_sn65dsi86_{read,write}_u16
> > >    drm/bridge: ti-sn65dsi86: Loosen coupling of PWM to ti-sn65dsi86 core
> > > 
> > >   drivers/gpu/drm/bridge/ti-sn65dsi86.c | 146 +++++++++++++++-----------
> > >   1 file changed, 83 insertions(+), 63 deletions(-)
> > > 
> > > base-commit: 4e87148f80d198ba5febcbcc969c6b9471099a09
> > 
> > It looks fine to me, even without the goal to move the driver to drivers/pwm
> > I think it's same to move the pwm ddata out of the main pdata ans associate
> > it to the pwm aux device lifetime.
> > 
> > I don't anything wrong, and so far it's of for me, let's see if there's comments
> > for other people before applying!
> 
> I like 1/3 very much, but as mentioned in a reply to 3/3, I'm not
> convinced by that one at all.

OK, I also wasn't completely sure that patches #2 and #3 are a good
idea. The benefit I see is only better separation of the two drivers
(which is very subjective) and making the main driver struct a bit
smaller, saving some memory if the PWM isn't bound (which (maybe) saves
very little memory in some corner cases only).

> Not only does it make the driver more
> complex for, I believe, very little gain (if any), usage of
> devm_kzalloc() in ti_sn_pwm_probe() is most likely wrong.

It's not more wrong that the same construct in nearly every other pwm
driver. With my big life-time series for pwm[1] that idiom is fine.

Best regards
Uwe

[1] https://lore.kernel.org/linux-pwm/20231121134901.208535-1-u.kleine-koenig@xxxxxxxxxxxxxx

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux