pinctrl: Fix resource leaks, out-of-bounds accesses, and error handling (batch 2)
Subsystems (8)
cover pinctrl: Fix resource leaks, out-of-bounds accesses, and error handling (batch 2)
These issues were reported by Sashiko (batch 2).
Lee Jones (10):
pinctrl: nuvoton: ma35: Unmap reg_base and release clock on probe
error
pinctrl: nuvoton: ma35: Prevent out-of-bounds write in
ma35_pinctrl_get_soc_data()
pinctrl: nuvoton: ma35: Assign fwnode to gpio_chip in
ma35_gpiolib_register()
pinctrl: nuvoton: ma35: Avoid leaking map when parent node is missing
pinctrl: nuvoton: npcm8xx: Propagate npcm8xx_gpio_register() error in
probe
pinctrl: nuvoton: npcm7xx: Fix fwnode_irq_get() error check in
npcm7xx_gpio_of() pinctrl: single: Reject invalid register width in pcs_probe()
pinctrl: single: Free pinconf settings array in pcs_dt_free_map()
pinctrl: sunplus: Validate pin_func index in sppctl_dt_node_to_map()
pinctrl: realtek: rtd1625: Set dcycle_offset to NA for pins without
duty-cycle support
drivers/pinctrl/nuvoton/pinctrl-ma35.c | 55 ++++++++++++++++++----- drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c | 7 +-- drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c | 2 +- drivers/pinctrl/pinctrl-single.c | 42 +++++++++-------- drivers/pinctrl/realtek/pinctrl-rtd1625.c | 12 ++--- drivers/pinctrl/sunplus/sppctl.c | 7 ++- 6 files changed, 85 insertions(+), 40 deletions(-)
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
PATCH 1/10 pinctrl: nuvoton: ma35: Unmap reg_base and release clock on probe error
ma35_get_bank_data() maps bank->reg_base via fwnode_iomap() and acquires and enables bank->clk via of_clk_get() and clk_prepare_enable(), neither of which is devres-managed. If of_clk_get() or clk_prepare_enable() fails, bank->reg_base is leaked, and ma35_pinctrl_get_soc_data() ignores the error and continues probing with an uninitialised bank.
Register devm cleanup actions for bank->reg_base and bank->clk, and propagate errors from ma35_get_bank_data() in ma35_pinctrl_get_soc_data().
--- drivers/pinctrl/nuvoton/pinctrl-ma35.c | 36 ++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-)
drivers/pinctrl/nuvoton/pinctrl-ma35.c +31 -5
@@ -555,6 +555,22 @@ static int ma35_gpiolib_register(struct platform_device *pdev, struct ma35_pinctreturn 0 ; } +static void ma35_bank_iounmap (void *data) +{ +struct ma35_pin_bank *bank = data; + +iounmap (bank->reg_base); + bank->reg_base =NULL ; +} + +static void ma35_bank_clk_release (void *data) +{ +struct ma35_pin_bank *bank = data; + +clk_disable_unprepare (bank->clk); +clk_put (bank->clk); +} +static int ma35_get_bank_data (struct ma35_pin_bank *bank) {int ret;
@@ -563,6 +579,10 @@ static int ma35_get_bank_data(struct ma35_pin_bank *bank)if (!bank->reg_base)return -ENOMEM ; + ret =devm_add_action_or_reset (bank->dev, ma35_bank_iounmap, bank); +if (ret) +return ret; + bank->irq =fwnode_irq_get (bank->fwnode,0 ); bank->nr_pins =MA35_GPIO_PORT_MAX ;
@@ -572,10 +592,12 @@ static int ma35_get_bank_data(struct ma35_pin_bank *bank)return PTR_ERR (bank->clk); ret =clk_prepare_enable (bank->clk); -if (ret) +if (ret) {clk_put (bank->clk); +return ret; + } -return ret; +return devm_add_action_or_reset (bank->dev, ma35_bank_clk_release, bank); }static int ma35_pinctrl_get_soc_data (struct ma35_pinctrl *pctl,struct platform_device *pdev)
@@ -583,7 +605,7 @@ static int ma35_pinctrl_get_soc_data(struct ma35_pinctrl *pctl, struct platform_struct fwnode_handle *child;struct ma35_pin_ctrl *ctrl;struct ma35_pin_bank *bank; -int i, id =0 ; +int i, id =0 , ret; ctrl = pctl->ctrl; ctrl->nr_banks =MA35_GPIO_BANK_MAX ;
@@ -603,8 +625,12 @@ static int ma35_pinctrl_get_soc_data(struct ma35_pinctrl *pctl, struct platform_ bank->fwnode = child; bank->regmap = pctl->regmap; bank->dev = &pdev->dev; -if (!ma35_get_bank_data (bank)) - bank->valid = true; + ret =ma35_get_bank_data (bank); +if (ret) { +fwnode_handle_put (child); +return ret; + } + bank->valid = true; id++; }return 0 ;
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
PATCH 2/10 pinctrl: nuvoton: ma35: Prevent out-of-bounds write in ma35_pinctrl_get_soc_data()
In reply to Lee Jonesma35_pinctrl_get_soc_data() allocates ctrl->pin_banks with ctrl->nr_banks (MA35_GPIO_BANK_MAX) entries and populates them while iterating over child GPIO controller nodes with for_each_gpiochip_node(). If the Device Tree contains more than MA35_GPIO_BANK_MAX GPIO controller child nodes, id exceeds ctrl->nr_banks and writes past the end of ctrl->pin_banks.
Check that id is within ctrl->nr_banks before indexing ctrl->pin_banks.
--- drivers/pinctrl/nuvoton/pinctrl-ma35.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/pinctrl/nuvoton/pinctrl-ma35.c +6 -0
@@ -621,6 +621,12 @@ static int ma35_pinctrl_get_soc_data(struct ma35_pinctrl *pctl, struct platform_ }for_each_gpiochip_node (&pdev->dev, child) { +if (id >= ctrl->nr_banks) { +dev_err (&pdev->dev,"Exceeded max GPIO banks (%u)\n" , + ctrl->nr_banks); +fwnode_handle_put (child); +return -EINVAL ; + } bank = &ctrl->pin_banks[id]; bank->fwnode = child; bank->regmap = pctl->regmap;
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
[email protected]
AI reviewRe: [PATCH 02/10] pinctrl: nuvoton: ma35: Prevent out-of-bounds write in ma35_pinctrl_get_soc_data()
Thank you for your contribution! Sashiko AI review found no new issues in this patch, but noticed 1 related pre-existing issue it does not introduce: - [Critical] pinctrl: nuvoton: use-after-free in…
Thank you for your contribution! Sashiko AI review found no new issues in this patch, but noticed 1 related pre-existing issue it does not introduce:
- [Critical] pinctrl: nuvoton: use-after-free in ma35_gpiolib_register()
[1]
--
commit d261fa88956097cc3c05a1bf0a8ef41b4bf318b2
Author: Lee Jones <[email protected]>
pinctrl: nuvoton: ma35: Prevent out-of-bounds write in ma35_pinctrl_get_soc_data()
This commit adds a bounds check on id against ctrl->nr_banks in ma35_pinctrl_get_soc_data() to prevent writing past the end of ctrl->pin_banks when the Device Tree contains more than MA35_GPIO_BANK_MAX GPIO controller child nodes.
Quoted 7 lines
drivers/pinctrl/nuvoton/pinctrl-ma35.c +0
-0@@ -622,6 +622,12 @@ static int ma35_pinctrl_get_soc_data(struct ma35_pinctrl *pctl, struct platform_ }
drivers/pinctrl/nuvoton/pinctrl-ma35.c static int ma35_pinctrl_get_soc_data(struct ma35_pinctrl *pctl, struct platform_ · line 633
Quoted 10 lines
drivers/pinctrl/nuvoton/pinctrl-ma35.c +6
-0for_each_gpiochip_node (&pdev->dev, child) { +if (id >= ctrl->nr_banks) { +dev_err (&pdev->dev,"Exceeded max GPIO banks (%u)\n" , + ctrl->nr_banks); +fwnode_handle_put (child); +return -EINVAL ; + } bank = &ctrl->pin_banks[id]; bank->fwnode = child; bank->regmap = pctl->regmap;
[Severity: Critical]
This problem wasn't introduced by this patch, but does storing child in bank->fwnode without fwnode_handle_get() leave a dangling pointer [1]?
for_each_gpiochip_node() drops the reference on each child node as the iterator advances, so bank->fwnode is left without a reference once the loop moves on. Later during probe, ma35_gpiolib_register() passes bank->fwnode to dev_warn() with "%pfw" if !bank->valid, resulting in a use-after-free.
[1] https://sashiko.dev/#/bug/linux-f140abe2-145a-4f6b-b626-bbabdc7327c4
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH 3/10 pinctrl: nuvoton: ma35: Assign fwnode to gpio_chip in ma35_gpiolib_register()
In reply to Lee Jonesma35_gpiolib_register() registers all 14 GPIO banks under the same parent platform device without setting bank->chip.fwnode to the child bank node. Consequently, all GPIO banks share the parent pinctrl node for OF/fwnode lookups, preventing consumer drivers from resolving phandles to individual GPIO bank child nodes (&gpiob, &gpioc, etc.).
Set bank->chip.fwnode to bank->fwnode before registering each GPIO chip.
--- drivers/pinctrl/nuvoton/pinctrl-ma35.c | 1 + 1 file changed, 1 insertion(+)
drivers/pinctrl/nuvoton/pinctrl-ma35.c +1 -0
@@ -518,6 +518,7 @@ static int ma35_gpiolib_register(struct platform_device *pdev, struct ma35_pinct bank->irqinten =0 ; bank->chip.label = bank->name; bank->chip.parent = &pdev->dev; + bank->chip.fwnode = bank->fwnode; bank->chip.request = ma35_gpio_core_to_request; bank->chip.direction_input = ma35_gpio_core_direction_in; bank->chip.direction_output = ma35_gpio_core_direction_out;
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
PATCH 4/10 pinctrl: nuvoton: ma35: Avoid leaking map when parent node is missing
In reply to Lee JonesCommit 3fd976afe974 ("pinctrl: nuvoton: fix a double free in ma35_pinctrl_dt_node_to_map_func()") removed the kfree(new_map) call on the !parent error path to avoid a double free, but left *map = new_map assigned before of_get_parent(np). Because pinctrl_dt_to_map() does not invoke dt_free_map() when dt_node_to_map() returns an error, new_map is leaked if of_get_parent(np) returns NULL.
Look up the parent node before allocating new_map, and release the parent node reference if the allocation fails.
--- drivers/pinctrl/nuvoton/pinctrl-ma35.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/pinctrl/nuvoton/pinctrl-ma35.c +7 -5
@@ -201,17 +201,19 @@ static int ma35_pinctrl_dt_node_to_map_func(struct pinctrl_dev *pctldev,return -EINVAL ; } + parent =of_get_parent (np); +if (!parent) +return -EINVAL ; + map_num += grp->grp.npins; new_map =kzalloc_objs (*new_map, map_num); -if (!new_map) +if (!new_map) { +of_node_put (parent);return -ENOMEM ; + } *map = new_map; *num_maps = map_num; -/* create mux map */ - parent =of_get_parent (np); -if (!parent) -return -EINVAL ; setting = grp->data;
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
[email protected]
AI reviewRe: [PATCH 04/10] pinctrl: nuvoton: ma35: Avoid leaking map when parent node is missing
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Low] pinctrl: nuvoton: ma35: inaccurate commit description of 3fd976afe974 --
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [Low] pinctrl: nuvoton: ma35: inaccurate commit description of 3fd976afe974
--
commit 16d0fceb87809d6ff503501ddb36d785b4f12a75
Author: Lee Jones <[email protected]>
pinctrl: nuvoton: ma35: Avoid leaking map when parent node is missing
This commit moves the parent device node lookup before allocating new_map in ma35_pinctrl_dt_node_to_map_func() so that new_map is not leaked if of_get_parent() returns NULL.
Quoted 4 lines
Commit
3fd976afe974("pinctrl: nuvoton: fix a double free inma35_pinctrl_dt_node_to_map_func()") removed thekfree(new_map)call on the !parent error path to avoid a double free, but left *map =new_mapassigned beforeof_get_parent(np).
[Severity: Low]
Is this description of 3fd976afe974 ("pinctrl: nuvoton: fix a double free in ma35_pinctrl_dt_node_to_map_func()") accurate?
There was never a kfree(new_map) call on the !parent error path. 3fd976afe974 ("pinctrl: nuvoton: fix a double free in ma35_pinctrl_dt_node_to_map_func()") switched the allocation from devm_kcalloc() to unmanaged kcalloc() without freeing new_map when of_get_parent() fails.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
PATCH 5/10 pinctrl: nuvoton: npcm8xx: Propagate npcm8xx_gpio_register() error in probe
In reply to Lee Jonesnpcm8xx_pinctrl_probe() logs an error with dev_err_probe() when npcm8xx_gpio_register() fails, but omits the return statement and returns 0 instead. Propagate the error returned by dev_err_probe().
--- drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c +1 -1
@@ -2583,7 +2583,7 @@ static int npcm8xx_pinctrl_probe(struct platform_device *pdev) ret =npcm8xx_gpio_register (pctrl);if (ret <0 ) -dev_err_probe (dev, ret,"Failed to register gpio\n" ); +return dev_err_probe (dev, ret,"Failed to register gpio\n" );return 0 ; }
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
[email protected]
AI reviewRe: [PATCH 05/10] pinctrl: nuvoton: npcm8xx: Propagate npcm8xx_gpio_register() error in probe
Thank you for your contribution! Sashiko AI review found no new issues in this patch, but noticed 1 related pre-existing issue it does not introduce: - [Medium] pinctrl: nuvoton: iomap leak in…
Thank you for your contribution! Sashiko AI review found no new issues in this patch, but noticed 1 related pre-existing issue it does not introduce:
- [Medium] pinctrl: nuvoton: iomap leak in npcm7xx_gpio_of() and npcm8xx_gpio_fw() [1]
--
commit 86095407b58f80f7127008d829c751165be4ce58
Author: Lee Jones <[email protected]>
pinctrl: nuvoton: npcm8xx: Propagate npcm8xx_gpio_register() error in probe
This commit adds the missing return statement in npcm8xx_pinctrl_probe() so that errors from npcm8xx_gpio_register() are propagated by dev_err_probe().
drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c static int npcm8xx_pinctrl_probe(struct platform_device *pdev) · line 2585
Quoted 10 lines
drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c +1
-1@@ -2582,7 +2582,7 @@ static int npcm8xx_pinctrl_probe(struct platform_device *pdev) ret =npcm8xx_gpio_register (pctrl);if (ret <0 ) -dev_err_probe (dev, ret,"Failed to register gpio\n" ); +return dev_err_probe (dev, ret,"Failed to register gpio\n" );
[Severity: Medium]
This problem wasn't introduced by this patch, but does returning an error here (or on earlier error paths in npcm8xx_pinctrl_probe() and npcm7xx_pinctrl_probe()) leak the pctrl->gpio_bank[id].base MMIO mappings [1]?
npcm8xx_gpio_fw() and npcm7xx_gpio_of() map each bank's registers with unmanaged fwnode_iomap(), and nothing calls iounmap() when a later bank initialization step, devm_pinctrl_register(), or GPIO registration fails:
drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c:npcm8xx_gpio_fw() {
...
for_each_gpiochip_node(dev, child) {
pctrl->gpio_bank[id].base = fwnode_iomap(child, 0);
if (!pctrl->gpio_bank[id].base)
return dev_err_probe(dev, -ENXIO, "fwnode_iomap id %d failed\n", id);
...
}[1] https://sashiko.dev/#/bug/linux-cf83b9c0-f3a2-4dc8-95a0-af6fafb6d092
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
PATCH 6/10 pinctrl: nuvoton: npcm7xx: Fix fwnode_irq_get() error check in npcm7xx_gpio_of()
In reply to Lee JonesCommit 693c9ecd8326 ("pinctrl: nuvoton: Reduce use of OF-specific APIs") replaced irq_of_parse_and_map(), which returns 0 on failure, with fwnode_irq_get(), which returns a negative errno on failure (including -EPROBE_DEFER), while leaving the !ret check unchanged. As a result, negative error codes are stored in pctrl->gpio_bank[id].irq as large unsigned values instead of aborting or deferring probe.
Check for ret < 0 and propagate the error code, matching the NPCM8XX driver.
--- drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c +4 -3
@@ -1856,9 +1856,10 @@ static int npcm7xx_gpio_of(struct npcm7xx_pinctrl *pctrl) } ret =fwnode_irq_get (child,0 ); -if (!ret) { -dev_err (dev,"No IRQ for GPIO bank %u\n" , id); -return -EINVAL ; +if (ret <0 ) { +fwnode_handle_put (child); +return dev_err_probe (dev, ret, +"No IRQ for GPIO bank %u\n" , id); } pctrl->gpio_bank[id].irq = ret; pctrl->gpio_bank[id].irqbase = id *NPCM7XX_GPIO_PER_BANK ;
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
PATCH 7/10 pinctrl: single: Reject invalid register width in pcs_probe()
In reply to Lee JonesIf the "pinctrl-single,register-width" property specifies an unsupported width (anything other than 8, 16, or 32), pcs_probe() hits the default branch in the switch statement and continues without initialising pcs->read or pcs->write, leading to a divide-by-zero in pcs_allocate_pin_table() or a NULL function pointer dereference when accessing registers.
Validate pcs->width immediately after reading the property and return -EINVAL from the default branch in pcs_probe().
--- drivers/pinctrl/pinctrl-single.c | 34 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/pinctrl/pinctrl-single.c +17 -17
@@ -1811,6 +1811,23 @@ static int pcs_probe(struct platform_device *pdev)return ret; } +switch (pcs->width) { +case 8 : + pcs->read = pcs_readb; + pcs->write = pcs_writeb; +break ; +case 16 : + pcs->read = pcs_readw; + pcs->write = pcs_writew; +break ; +case 32 : + pcs->read = pcs_readl; + pcs->write = pcs_writel; +break ; +default : +return -EINVAL ; + } + ret =of_property_read_u32 (np,"pinctrl-single,function-mask" , &pcs->fmask);if (!ret) {
@@ -1860,23 +1877,6 @@ static int pcs_probe(struct platform_device *pdev)platform_set_drvdata (pdev, pcs); -switch (pcs->width) { -case 8 : - pcs->read = pcs_readb; - pcs->write = pcs_writeb; -break ; -case 16 : - pcs->read = pcs_readw; - pcs->write = pcs_writew; -break ; -case 32 : - pcs->read = pcs_readl; - pcs->write = pcs_writel; -break ; -default : -break ; - } - pcs->desc.name =DRIVER_NAME ; pcs->desc.pctlops = &pcs_pinctrl_ops; pcs->desc.pmxops = &pcs_pinmux_ops;
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
PATCH 8/10 pinctrl: single: Free pinconf settings array in pcs_dt_free_map()
In reply to Lee Jonespcs_parse_pinconf() allocates a settings array via devm_kcalloc() on the pin controller device and attaches it to map[1].data.configs.configs. When the pinctrl core copies the map entries into its internal tables or tears down a consumer device mapping, it invokes pcs_dt_free_map(), which only frees the outer map array and leaves the per-map configs array allocated on pcs->dev until the pin controller itself is unbound.
Free map[i].data.configs.configs for PIN_MAP_TYPE_CONFIGS_GROUP entries in pcs_dt_free_map().
--- drivers/pinctrl/pinctrl-single.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pinctrl/pinctrl-single.c +6 -2
@@ -311,9 +311,13 @@ static void pcs_pin_dbg_show(struct pinctrl_dev *pctldev,static void pcs_dt_free_map (struct pinctrl_dev *pctldev,struct pinctrl_map *map,unsigned num_maps) { -struct pcs_device *pcs; +struct pcs_device *pcs =pinctrl_dev_get_drvdata (pctldev); + +for (unsigned int i =0 ; i < num_maps; i++) { +if (map[i].type ==PIN_MAP_TYPE_CONFIGS_GROUP ) +devm_kfree (pcs->dev, map[i].data.configs.configs); + } - pcs =pinctrl_dev_get_drvdata (pctldev);devm_kfree (pcs->dev, map); }
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
PATCH 9/10 pinctrl: sunplus: Validate pin_func index in sppctl_dt_node_to_map()
In reply to Lee JonesIn sppctl_dt_node_to_map(), the validation loop checks that pin_num is within sppctl_pins_all_sz, but does not validate pin_func (bits 15:8) before indexing sppctl_list_funcs[pin_func] for fully-pinmuxed pins. Check that pin_func is less than sppctl_list_funcs_sz when pin_type selects a mux function.
--- drivers/pinctrl/sunplus/sppctl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/pinctrl/sunplus/sppctl.c +6 -1
@@ -867,8 +867,13 @@ static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_nodefor (i =0 ; i < (*num_maps); i++) { dt_pin =be32_to_cpu (list[i]); pin_num =FIELD_GET (GENMASK (31 ,24 ), dt_pin); + pin_type =FIELD_GET (GENMASK (23 ,16 ), dt_pin); + pin_func =FIELD_GET (GENMASK (15 ,8 ), dt_pin); -if (pin_num >= sppctl_pins_all_sz) { +if (pin_num >= sppctl_pins_all_sz || + (pin_type !=SPPCTL_PCTL_G_GPIO && + pin_type !=SPPCTL_PCTL_G_IOPP && + pin_func >= sppctl_list_funcs_sz)) {dev_err (pctldev->dev,"Invalid pin property at index %d (0x%08x)\n" , i, dt_pin);return -EINVAL ;
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
Re: [PATCH 09/10] pinctrl: sunplus: Validate pin_func index in sppctl_dt_node_to_map()
Quoted 11 lines · Lee Jones
In
sppctl_dt_node_to_map(), the validation loop checks thatpin_numis withinsppctl_pins_all_sz, but does not validatepin_func(bits 15:8) before indexing sppctl_list_funcs[pin_func] for fully-pinmuxed pins. Check thatpin_funcis less thansppctl_list_funcs_szwhenpin_typeselects a mux function.Fixes aa74c44be19c ("pinctrl: Add driver for Sunplus SP7021")Signed-off-by Lee Jones <[email protected]>---
PATCH 10/10 pinctrl: realtek: rtd1625: Set dcycle_offset to NA for pins without duty-cycle support
In reply to Lee JonesIn rtd1625_main2_sconfigs[], pins gpio_86 through gpio_91 do not support clock duty-cycle adjustment (dcycle_maskbits is 0), but specify 0 instead of NA for dcycle_offset. Because rtd_pconf_parse_conf() tests sconfig_desc->dcycle_offset == NA to determine whether duty-cycle configuration is supported, an offset of 0 causes it to compute GENMASK(0 - 1, 0) and corrupt the pad control register.
Set dcycle_offset to NA for gpio_86 through gpio_91.
--- drivers/pinctrl/realtek/pinctrl-rtd1625.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/pinctrl/realtek/pinctrl-rtd1625.c +6 -6
@@ -2993,12 +2993,12 @@ static const struct rtd_pin_sconfig_desc rtd1625_main2_sconfigs[] = {RTK_PIN_SCONFIG (gpio_65,0x34 ,16 ,3 ,19 ,3 ,22 ,3 ),RTK_PIN_SCONFIG (gpio_66,0x38 ,3 ,3 ,6 ,3 ,9 ,3 ),RTK_PIN_SCONFIG (gpio_67,0x38 ,16 ,3 ,19 ,3 ,22 ,3 ), -RTK_PIN_SCONFIG (gpio_86,0x3c ,0 ,0 ,14 ,4 ,18 ,4 ), -RTK_PIN_SCONFIG (gpio_87,0x3c ,0 ,0 ,24 ,4 ,28 ,4 ), -RTK_PIN_SCONFIG (gpio_88,0x40 ,0 ,0 ,2 ,4 ,6 ,4 ), -RTK_PIN_SCONFIG (gpio_89,0x40 ,0 ,0 ,12 ,4 ,16 ,4 ), -RTK_PIN_SCONFIG (gpio_90,0x40 ,0 ,0 ,22 ,4 ,26 ,4 ), -RTK_PIN_SCONFIG (gpio_91,0x44 ,0 ,0 ,2 ,4 ,6 ,4 ), +RTK_PIN_SCONFIG (gpio_86,0x3c ,NA ,0 ,14 ,4 ,18 ,4 ), +RTK_PIN_SCONFIG (gpio_87,0x3c ,NA ,0 ,24 ,4 ,28 ,4 ), +RTK_PIN_SCONFIG (gpio_88,0x40 ,NA ,0 ,2 ,4 ,6 ,4 ), +RTK_PIN_SCONFIG (gpio_89,0x40 ,NA ,0 ,12 ,4 ,16 ,4 ), +RTK_PIN_SCONFIG (gpio_90,0x40 ,NA ,0 ,22 ,4 ,26 ,4 ), +RTK_PIN_SCONFIG (gpio_91,0x44 ,NA ,0 ,2 ,4 ,6 ,4 ),RTK_PIN_SCONFIG (hif_clk,0x44 ,13 ,3 ,16 ,3 ,19 ,3 ),RTK_PIN_SCONFIG (hif_data,0x48 ,3 ,3 ,6 ,3 ,9 ,3 ), RTK_PIN_SCONFIG(hif_en, 0x48, 16, 3, 19, 3, 22, 3),
Signature · 1 line
2.56.0.567.gdb80ae8cd5-goog
RE: [PATCH 10/10] pinctrl: realtek: rtd1625: Set dcycle_offset to NA for pins without duty-cycle support
Quoted 7 lines
In rtd1625_main2_sconfigs[], pins
gpio_86throughgpio_91do not support clock duty-cycle adjustment (dcycle_maskbitsis 0), but specify 0 instead of NA fordcycle_offset. Becausertd_pconf_parse_conf()testssconfig_desc->dcycle_offset== NA to determine whether duty-cycle configuration is supported, an offset of 0 causes it to computeGENMASK(0 - 1, 0)and corrupt the pad control register.
Quoted 7 lines
Set
dcycle_offsetto NA forgpio_86throughgpio_91.Fixes e309dbd523b8 ("pinctrl: realtek: add rtd1625 pinctrl driver")Closes:
https://sashiko.dev/#/bug/linux-8a8f690f-15e7-4dbd-a3a2-87bc00edf7b9Signed-off-by Lee Jones <[email protected]>
Thanks for the patch!
Quoted 31 lines
--- drivers/pinctrl/realtek/pinctrl-rtd1625.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)drivers/pinctrl/realtek/pinctrl-rtd1625.c +6
-6@@ -2993,12 +2993,12 @@ static const struct rtd_pin_sconfig_desc rtd1625_main2_sconfigs[] = { RTK_PIN_SCONFIG(gpio_65, 0x34, 16, 3, 19, 3, 22, 3), RTK_PIN_SCONFIG(gpio_66, 0x38, 3, 3, 6, 3, 9, 3), RTK_PIN_SCONFIG(gpio_67, 0x38, 16, 3, 19, 3, 22, 3), - RTK_PIN_SCONFIG(gpio_86, 0x3c, 0, 0, 14, 4, 18, 4), - RTK_PIN_SCONFIG(gpio_87, 0x3c, 0, 0, 24, 4, 28, 4), - RTK_PIN_SCONFIG(gpio_88, 0x40, 0, 0, 2, 4, 6, 4), - RTK_PIN_SCONFIG(gpio_89, 0x40, 0, 0, 12, 4, 16, 4), - RTK_PIN_SCONFIG(gpio_90, 0x40, 0, 0, 22, 4, 26, 4), - RTK_PIN_SCONFIG(gpio_91, 0x44, 0, 0, 2, 4, 6, 4), + RTK_PIN_SCONFIG(gpio_86, 0x3c, NA, 0, 14, 4, 18, 4), + RTK_PIN_SCONFIG(gpio_87, 0x3c, NA, 0, 24, 4, 28, 4), + RTK_PIN_SCONFIG(gpio_88, 0x40, NA, 0, 2, 4, 6, 4), + RTK_PIN_SCONFIG(gpio_89, 0x40, NA, 0, 12, 4, 16, 4), + RTK_PIN_SCONFIG(gpio_90, 0x40, NA, 0, 22, 4, 26, 4), + RTK_PIN_SCONFIG(gpio_91, 0x44, NA, 0, 2, 4, 6, 4), RTK_PIN_SCONFIG(hif_clk, 0x44, 13, 3, 16, 3, 19, 3), RTK_PIN_SCONFIG(hif_data, 0x48, 3, 3, 6, 3, 9, 3),RTK_PIN_SCONFIG(hif_en, 0x48, 16, 3, 19, 3, 22, 3),--
2.56.0.567.gdb80ae8cd5-goog