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
