Hi, On Thu, Nov 23, 2023 at 9:54 AM Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > Introduce a dedicated private data structure for the pwm aux driver > provided by the sn65dsi86 driver. This way data needed for PWM operation > is (to a certain degree) nicely separated and doesn't occupy memory in > the ti_sn65dsi86 core's private data if the PWM isn't used. I suspect we still end up at a loss memory-wise. All of the extra code + the overhead of another kmalloc seems like it would take up more space than the tiny bit of data in the structure. > The eventual goal is to decouple the PWM driver completely from the > ti-sn65dsi86 core and maybe even move it to a dedicated driver below > drivers/pwm. There are a few obstacles to that quest though: > > - The busy pin check (implemented in ti_sn_pwm_pin_request() and > ti_sn_pwm_pin_release()) would need to be available unconditionally. > > - The refclk should probably abstracted by a struct clk such that the > pwm_refclk_freq member that currently still lives in ti-sn65dsi86 > core driver data can be dropped. Right that the above could be done with more abstraction layers. I guess the question I have is: how much do we gain with that? Personally I'm not really sold on the idea. If others think this is a great change then I won't stand in the way, but IMO without a compelling reason this is just extra abstraction / complexity without any gain... > +/* > + * struct ti_sn65dsi86_pwm_ddata - Platform data for ti-sn65dsi86 pwm driver. Why "ddata" exactly? It seems like this is just the pwm "data" ? > + * @chip: pwm_chip if the PWM is exposed. > + * @pwm_enabled: Used to track if the PWM signal is currently enabled. > + * @regmap: Regmap for accessing i2c. > + * @pdata: platform data of the parent device "pdata" isn't a member of the struct, but "pwm_refclk_freq" is. -Doug
