[Letux-kernel] New LetuxOS Kernels - strcmp(NULL)

H. Nikolaus Schaller hns at goldelico.com
Thu Jun 21 19:16:48 CEST 2018


Hi Tony,

> Am 21.06.2018 um 08:42 schrieb Tony Lindgren <tony at atomide.com>:
> 
> * H. Nikolaus Schaller <hns at goldelico.com> [180620 15:36]:
>> [    7.154541] i2c 1-0030: Retrying from deferred list
>> [    7.164642] pinctrl_generic_add_group: pctldev=ee3bc200 name=pinmux_camera_pins found selector=19
>> [    7.174285] w1_master_driver w1_bus_master1: Attaching one wire slave 01.000000000000 crc 3d
>> [    7.196868] pcs_dt_free_map
>> [    7.205200] i2c 1-0030: Retrying from deferred list
>> [    7.217590] bq27xxx_battery_setup
>> [    7.225982] pinctrl_generic_add_group: pctldev=ee3bc200 name=pinmux_camera_pins found selector=19
>> [    7.235565] bq27xxx_battery_setup: dm_regs=  (null)
>> [    7.248168] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix
> 
> Hmm what's up with this warning above? Is that NULL related to
> pinctrl issue?
> 
>> [    7.257446] bq27xxx_battery_settings
>> [    7.261535] pcs_dt_free_map
>> [    7.274200] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-1088290796
>> [    7.322113] pcs_dt_free_map
>> [    7.356658] i2c 1-0030: Retrying from deferred list
> 
> 
>> [    7.363830] (NULL device *): hwmon: 'gta04-battery' is not a valid name attribute, please fix
>> [    7.373260] pinctrl_generic_add_group: pctldev=ee3bc200 name=pinmux_camera_pins found selector=19
>> [    7.386840] pcs_dt_free_map
>> [    7.394378] platform backlight: Retrying from deferred list
>> [    7.406372] pinctrl_generic_get_group_name: group>name is NULL
> 
> So what's the selector number here when group->name == NULL?
> Still 19 (or again 19 for a new entry) or something else?

I tried to answer this - but today is a bad day to see strcmp(NULL)...

But adding more printk did show something strange:

root at letux:~# dmesg|fgrep generic_add
[    0.759002] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_hsusb2_pins selector=0
[    0.760894] pinctrl_generic_add_group: pctldev=ee487f80 name=pinmux_hsusb2_2_pins selector=0
[    0.761627] pinctrl_generic_add_group: pctldev=ee487e00 name=pinmux_mcbsp1_devconf0_pins selector=0
[    0.762451] pinctrl_generic_add_group: pctldev=ee487d00 name=pinmux_tv_acbias_devconf1_pins selector=0
[    0.779693] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_uart1_pins selector=1
[    0.781463] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_uart2_pins selector=2
[    0.782592] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_uart3_pins selector=3
[    1.809814] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_twl4030_pins selector=4
[    1.819122] pinctrl_generic_add_group: pctldev=ee37b000 name=pinmux_twl4030_vpins selector=0
[    1.982299] pinctrl_generic_add_group: pctldev=ee487f80 name=spi_gpio_pinmux selector=1
[    2.178070] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_mmc1_pins selector=5
[    2.214752] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_wlan_irq_pin selector=6
[    2.392944] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_gpmc_pins selector=7
[    2.787231] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_wlan_pins selector=8
[    2.804107] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_bt_pins selector=9
[    2.820159] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_wlan_irq_pin found selector=10
[    5.488616] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_dss_dpi_pins selector=10
[    6.313049] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_mcbsp1_pins selector=11
[    6.363708] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_penirq_pins selector=12
[    6.417938] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_mcbsp2_pins selector=13
[    6.597686] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins selector=14
[    6.766235] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=15
[    6.792480] pinctrl_generic_add_group: pctldev=ee37b180 name=hdq_pins selector=15
[    6.853607] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=16
[    6.892120] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=16
[    6.973480] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=16
[    7.012908] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_mcbsp3_pins selector=16
[    7.092163] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=17
[    7.122650] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_mcbsp4_pins selector=17
[    7.197998] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=18
[    7.238037] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=18
[    7.331420] pinctrl_generic_add_group: pctldev=ee37b180 name=backlight_pins_pinmux selector=18
[    7.541961] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=19
[    7.570434] pinctrl_generic_add_group: pctldev=ee37b180 name=modem_pins selector=19
[    7.617340] pinctrl_generic_add_group: pctldev=ee37b180 name=backlight_pins_pinmux found selector=20
[    7.675689] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=20
[    8.013305] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=20
[    8.046417] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=20
[    8.338043] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=20
[    8.435729] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=20
[    8.461212] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_pps_pins selector=20
[    8.554809] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=21
[    9.554016] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=21
[    9.781677] pinctrl_generic_add_group: pctldev=ee37b180 name=pinmux_camera_pins found selector=21
root at letux:~# 

The code is:

static int pinctrl_generic_group_name_to_selector(struct pinctrl_dev *pctldev,
						  const char *function)
{
	const struct pinctrl_ops *ops = pctldev->desc->pctlops;
	unsigned ngroups = ops->get_groups_count(pctldev);
	unsigned selector = 0;

//	printk("%s: function=%s%d", __func__, name);

	/* See if this pctldev has this group */
	while (selector < ngroups) {
		const char *gname = ops->get_group_name(pctldev, selector);

if (!gname || !function) {
	printk("%s: pctldev = %px function = %s\n", __func__, pctldev, function);
	printk(" strcmp(%s, %s)\n", __func__, function, gname);
	printk("  get_group_name() = %pF\n", ops->get_group_name);
	printk("  ngroups = %u\n", ngroups);
	printk("  selector = %u\n", selector);
	// if (pctldev->dev)
	//	printk("  pctldev->dev.name = %ps\n", pctldev->dev->name);
	dump_stack();
}

		if (!strcmp(function, gname))
			return selector;

		selector++;
	}

	return -EINVAL;
}

/**
 * pinctrl_generic_add_group() - adds a new pin group
 * @pctldev: pin controller device
 * @name: name of the pin group
 * @pins: pins in the pin group
 * @num_pins: number of pins in the pin group
 * @data: pin controller driver specific data
 *
 * Note that the caller must take care of locking.
 */
int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name,
			      int *pins, int num_pins, void *data)
{
	struct group_desc *group;
	int selector;
	// int err;

	selector = pinctrl_generic_group_name_to_selector(pctldev, name);
	if (selector >= 0) {
		printk("%s: pctldev=%px name=%s found selector=%d\n", __func__, pctldev, name, pctldev->num_groups);
		return selector;
	}

	selector = pctldev->num_groups;

	group = devm_kzalloc(pctldev->dev, sizeof(*group), GFP_KERNEL);
	if (!group)
		return -ENOMEM;

	printk("%s: pctldev=%px name=%s selector=%d\n", __func__, pctldev, name, pctldev->num_groups);

// if(strcmp(name, "pinmux_camera_pins") == 0) dump_stack();	// who calls me?

	group->name = name;
	group->pins = pins;
	group->num_pins = num_pins;
	group->data = data;

	// err =
	radix_tree_insert(&pctldev->pin_group_tree, selector,
			  group);

	// if (err)
	//	printk("%s: radix tree error %d\n", __func__, err);

	pctldev->num_groups++;

	return selector;
}
EXPORT_SYMBOL_GPL(pinctrl_generic_add_group);

The strange thing is that "pinmux_camera_pins found" increases every now
and then which means that the slot index (aka selector) in the radix tree
is renumbered.

So this means this happens:

* a driver probes
* it gets an entry in the radix tree
* deferred probing fails
* the entry stays in the radix tree
* the next driver being probed does insert at the same index
* but the existing entry stays and is shifted to an index one higher
* if code is running concurrently there may be a mismatch of selector numbers

An idea to fix it: always append to the real and of the tree and ignore
pctldev->num_groups when assigning a new entry.

That could mean replacing 

	selector = pctldev->num_groups;
	...
	pctldev->num_groups++;

by

	selector = ops->get_groups_count(pctldev);

This means we always assign a selector one higher than the last one that
exists.

Unfortunately I can't test this idea if it solves the strcmp(NULL) issue
on my current setup.

What do you think?

BR,
Nikolaus



More information about the Letux-kernel mailing list