[Letux-kernel] New LetuxOS Kernels - strcmp(NULL)
H. Nikolaus Schaller
hns at goldelico.com
Thu Jun 21 19:45:21 CEST 2018
> Am 21.06.2018 um 19:16 schrieb H. Nikolaus Schaller <hns at goldelico.com>:
>
> 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);
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 at openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel
More information about the Letux-kernel
mailing list