[PATCH v2 1/8] pwm: mediatek: Simplify representation of channel offsets
Uwe Kleine-König
u.kleine-koenig at baylibre.com
Mon Aug 4 06:14:14 PDT 2025
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).
Best regards
Uwe
---->8----
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index e4b595fc5a5e..ae8fa561ce2c 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -34,11 +34,38 @@
#define PWM_CLK_DIV_MAX 7
+struct pwm_mediatek_regs {
+ u16 pwm_ck_26m_sel_reg;
+ u16 chan_base;
+ u16 chan_width;
+};
+
+struct pwm_mediatek_regs pwm_mediatek_v1_reg_data = {
+ .chan_base = 0x10,
+ .chan_width = 0x40,
+};
+
+struct pwm_mediatek_regs pwm_mediatek_v1_26m_reg_data = {
+ .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
+ .chan_base = 0x10,
+ .chan_width = 0x40,
+};
+
+struct pwm_mediatek_regs pwm_mediatek_v2_reg_data = {
+ .chan_base = 0x80,
+ .chan_width = 0x40,
+};
+
+struct pwm_mediatek_regs pwm_mediatek_v3_26m_reg_data = {
+ .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL_V3,
+ .chan_base = 0x100,
+ .chan_width = 0x100,
+};
+
struct pwm_mediatek_of_data {
unsigned int num_pwms;
bool pwm45_fixup;
- u16 pwm_ck_26m_sel_reg;
- const unsigned int *reg_offset;
+ const struct pwm_mediatek_regs *reg_data;
};
/**
@@ -57,19 +84,6 @@ struct pwm_mediatek_chip {
const struct pwm_mediatek_of_data *soc;
};
-static const unsigned int mtk_pwm_reg_offset_v1[] = {
- 0x0010, 0x0050, 0x0090, 0x00d0, 0x0110, 0x0150, 0x0190, 0x0220
-};
-
-static const unsigned int mtk_pwm_reg_offset_v2[] = {
- 0x0080, 0x00c0, 0x0100, 0x0140, 0x0180, 0x01c0, 0x0200, 0x0240
-};
-
-/* PWM IP Version 3.0.2 */
-static const unsigned int mtk_pwm_reg_offset_v3[] = {
- 0x0100, 0x0200, 0x0300, 0x0400, 0x0500, 0x0600, 0x0700, 0x0800
-};
-
static inline struct pwm_mediatek_chip *
to_pwm_mediatek_chip(struct pwm_chip *chip)
{
@@ -118,7 +132,7 @@ static inline void pwm_mediatek_writel(struct pwm_mediatek_chip *chip,
unsigned int num, unsigned int offset,
u32 value)
{
- writel(value, chip->regs + chip->soc->reg_offset[num] + offset);
+ writel(value, chip->regs + chip->soc->reg_data->chan_base + num * chip->soc->reg_data->chan_width + offset);
}
static void pwm_mediatek_enable(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -162,8 +176,8 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
}
/* Make sure we use the bus clock and not the 26MHz clock */
- if (pc->soc->pwm_ck_26m_sel_reg)
- writel(0, pc->regs + pc->soc->pwm_ck_26m_sel_reg);
+ if (pc->soc->reg_data->pwm_ck_26m_sel_reg)
+ writel(0, pc->regs + pc->soc->reg_data->pwm_ck_26m_sel_reg);
/* Using resolution in picosecond gets accuracy higher */
resolution = (u64)NSEC_PER_SEC * 1000;
@@ -303,86 +317,79 @@ static int pwm_mediatek_probe(struct platform_device *pdev)
static const struct pwm_mediatek_of_data mt2712_pwm_data = {
.num_pwms = 8,
.pwm45_fixup = false,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_reg_data,
};
static const struct pwm_mediatek_of_data mt6795_pwm_data = {
.num_pwms = 7,
.pwm45_fixup = false,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_reg_data,
};
static const struct pwm_mediatek_of_data mt7622_pwm_data = {
.num_pwms = 6,
.pwm45_fixup = false,
- .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_26m_reg_data,
};
static const struct pwm_mediatek_of_data mt7623_pwm_data = {
.num_pwms = 5,
.pwm45_fixup = true,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_reg_data,
};
static const struct pwm_mediatek_of_data mt7628_pwm_data = {
.num_pwms = 4,
.pwm45_fixup = true,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_reg_data,
};
static const struct pwm_mediatek_of_data mt7629_pwm_data = {
.num_pwms = 1,
.pwm45_fixup = false,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_reg_data,
};
static const struct pwm_mediatek_of_data mt7981_pwm_data = {
.num_pwms = 3,
.pwm45_fixup = false,
- .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
- .reg_offset = mtk_pwm_reg_offset_v2,
+ .reg_data = &pwm_mediatek_v1_26m_reg_data,
};
static const struct pwm_mediatek_of_data mt7986_pwm_data = {
.num_pwms = 2,
.pwm45_fixup = false,
- .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_26m_reg_data,
};
static const struct pwm_mediatek_of_data mt7988_pwm_data = {
.num_pwms = 8,
.pwm45_fixup = false,
- .reg_offset = mtk_pwm_reg_offset_v2,
+ .reg_data = &pwm_mediatek_v2_reg_data,
};
static const struct pwm_mediatek_of_data mt8183_pwm_data = {
.num_pwms = 4,
.pwm45_fixup = false,
- .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_26m_reg_data,
};
static const struct pwm_mediatek_of_data mt8365_pwm_data = {
.num_pwms = 3,
.pwm45_fixup = false,
- .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_26m_reg_data,
};
static const struct pwm_mediatek_of_data mt8516_pwm_data = {
.num_pwms = 5,
.pwm45_fixup = false,
- .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL,
- .reg_offset = mtk_pwm_reg_offset_v1,
+ .reg_data = &pwm_mediatek_v1_26m_reg_data,
};
static const struct pwm_mediatek_of_data mt6991_pwm_data = {
.num_pwms = 4,
.pwm45_fixup = false,
- .pwm_ck_26m_sel_reg = PWM_CK_26M_SEL_V3,
- .reg_offset = mtk_pwm_reg_offset_v3,
+ .reg_data = &pwm_mediatek_v3_26m_reg_data,
};
static const struct of_device_id pwm_mediatek_of_match[] = {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-mediatek/attachments/20250804/036a7621/attachment.sig>
More information about the Linux-mediatek
mailing list