[PATCH v2 1/8] pwm: mediatek: Simplify representation of channel offsets

[AngeloGioacchino Del Regno]

Il 04/08/25 15:14, Uwe Kleine-König ha scritto:
> On Mon, Aug 04, 2025 at 12:30:45PM +0200, Uwe Kleine-König wrote:
>> Hallo AngeloGioacchino,
>>
>> On Mon, Aug 04, 2025 at 10:50:01AM +0200, AngeloGioacchino Del Regno wrote:
>>> Il 25/07/25 17:45, Uwe Kleine-König ha scritto:
>>>> The general register layout contains some per-chip registers starting at
>>>> offset 0 and then at a higher address there are n nearly identical and
>>>> equidistant blocks for the registers of the n channels.
>>>>
>>>> This allows to represent the offsets of per-channel registers as $base +
>>>> i * $width instead of listing all (or too many) offsets explicitly in an
>>>> array. So for a small additional effort in pwm_mediatek_writel() the
>>>> three arrays with the channel offsets can be dropped.
>>>>
>>>> The size changes according to bloat-o-meter are:
>>>>
>>>> 	add/remove: 0/3 grow/shrink: 1/0 up/down: 12/-96 (-84)
>>>> 	Function                                     old     new   delta
>>>> 	pwm_mediatek_apply                           696     708     +12
>>>> 	mtk_pwm_reg_offset_v3                         32       -     -32
>>>> 	mtk_pwm_reg_offset_v2                         32       -     -32
>>>> 	mtk_pwm_reg_offset_v1                         32       -     -32
>>>> 	Total: Before=5347, After=5263, chg -1.57%
>>>>
>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig at baylibre.com>
>>>
>>> What if we do, instead...
>>>
>>> struct pwm_mediatek_regs {
>>> 	u16 pwm_ck_26m_sel_reg;
>>> 	u16 chan_base;
>>> 	u16 chan_width;
>>> };
>>>
>>> struct pwm_mediatek_regs pwm_v1_reg_data = {
>>> 	.pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
>>> 	.chan_base = 0x10,
>>> 	.chan_width = 0x40,
>>> };
>>>
>>> static const struct pwm_mediatek_of_data mt7986_pwm_data = {
>>> 	....
>>> 	.reg_data = &pwm_v1_reg_data,
>>> };
>>>
>>> ...that should reduce the bloat even more :-)
>>
>> Having the three u16 directly in pwm_mediatek_of_data is cheaper because
>> .reg_data is a pointer and so 64 bits wide (on arm64) and so bigger than
>> 3xu16. Also having the data directly in pwm_mediatek_of_data saves one
>> indirection and so it should also be slightly faster.
> 
> I took the time to complete your patch suggestion and the bloat-o-meter
> output is:
> 
> add/remove: 4/3 grow/shrink: 1/0 up/down: 56/-96 (-40)
> Function                                     old     new   delta
> pwm_mediatek_apply                           776     808     +32
> pwm_mediatek_v3_26m_reg_data                   -       6      +6
> pwm_mediatek_v2_reg_data                       -       6      +6
> pwm_mediatek_v1_reg_data                       -       6      +6
> pwm_mediatek_v1_26m_reg_data                   -       6      +6
> mtk_pwm_reg_offset_v3                         32       -     -32
> mtk_pwm_reg_offset_v2                         32       -     -32
> mtk_pwm_reg_offset_v1                         32       -     -32
> Total: Before=5427, After=5387, chg -0.74%
> 
> See below for the complete patch for reference.
> 
> I tend to stick to my variant, also because then all info is in a single
> struct which is nice for both the human reader and the generated code
> (only on indirection).
> 

With this amount of reasoning, I can only say ...

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno at collabora.com>

;-)

Cheers!
Angelo

> Best regards
> Uwe
>