Am 21.06.2018 um 19:16 schrieb H. Nikolaus Schaller hns@goldelico.com:
Hi Tony,
Am 21.06.2018 um 08:42 schrieb Tony Lindgren tony@atomide.com:
- H. Nikolaus Schaller hns@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@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@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);
well, my printk isn't correct! I'll check if the result is just an effect of the wrong printk.
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
http://projects.goldelico.com/p/gta04-kernel/ Letux-kernel mailing list Letux-kernel@openphoenux.org http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel