Message ID | 1465766027-485-1-git-send-email-kieran@bingham.xyz |
---|---|
State | Not Applicable |
Headers | show |
Hi, [auto build test ERROR on abelloni/rtc-next] [also build test ERROR on v4.7-rc3 next-20160609] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Kieran-Bingham/rtc-convert-ds1307-to-interim-probe_new/20160613-051618 base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next config: sparc64-allmodconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sparc64 All error/warnings (new ones prefixed by >>): drivers/rtc/rtc-ds1307.c: In function 'ds1307_probe': >> drivers/rtc/rtc-ds1307.c:1262:9: error: implicit declaration of function 'i2c_of_match_device' [-Werror=implicit-function-declaration] idof = i2c_of_match_device(ds1307_dt_ids, client); ^ >> drivers/rtc/rtc-ds1307.c:1262:7: warning: assignment makes pointer from integer without a cast [-Wint-conversion] idof = i2c_of_match_device(ds1307_dt_ids, client); ^ >> drivers/rtc/rtc-ds1307.c:1269:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] chip = &chips[(int)idof->data]; ^ drivers/rtc/rtc-ds1307.c:1274:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] ds1307->type = (int) idof->data; ^ drivers/rtc/rtc-ds1307.c: At top level: >> drivers/rtc/rtc-ds1307.c:1634:2: error: unknown field 'probe_new' specified in initializer .probe_new = ds1307_probe, ^ >> drivers/rtc/rtc-ds1307.c:1634:15: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] .probe_new = ds1307_probe, ^ drivers/rtc/rtc-ds1307.c:1634:15: note: (near initialization for 'ds1307_driver.id_table') cc1: some warnings being treated as errors vim +/i2c_of_match_device +1262 drivers/rtc/rtc-ds1307.c 1256 1257 ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL); 1258 if (!ds1307) 1259 return -ENOMEM; 1260 1261 /* If we've got this far, this shouldn't be able to fail - but check anyway for now */ > 1262 idof = i2c_of_match_device(ds1307_dt_ids, client); 1263 if (!idof) { 1264 dev_err(&client->dev, "Probe failed to find an id entry\n"); 1265 return -ENODEV; 1266 } 1267 1268 /* Now we can set our chip entry */ > 1269 chip = &chips[(int)idof->data]; 1270 1271 i2c_set_clientdata(client, ds1307); 1272 1273 ds1307->client = client; > 1274 ds1307->type = (int) idof->data; 1275 1276 if (!pdata && client->dev.of_node) 1277 ds1307_trickle_of_init(client, chip); --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi, [auto build test WARNING on abelloni/rtc-next] [also build test WARNING on v4.7-rc3 next-20160609] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Kieran-Bingham/rtc-convert-ds1307-to-interim-probe_new/20160613-051618 base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next config: tile-allmodconfig (attached as .config) compiler: tilegx-linux-gcc (GCC) 4.6.2 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=tile All warnings (new ones prefixed by >>): drivers/rtc/rtc-ds1307.c: In function 'ds1307_probe': drivers/rtc/rtc-ds1307.c:1262:2: error: implicit declaration of function 'i2c_of_match_device' drivers/rtc/rtc-ds1307.c:1262:7: warning: assignment makes pointer from integer without a cast [enabled by default] drivers/rtc/rtc-ds1307.c:1269:16: warning: cast from pointer to integer of different size drivers/rtc/rtc-ds1307.c:1274:17: warning: cast from pointer to integer of different size drivers/rtc/rtc-ds1307.c: At top level: drivers/rtc/rtc-ds1307.c:1634:2: error: unknown field 'probe_new' specified in initializer >> drivers/rtc/rtc-ds1307.c:1634:2: warning: initialization from incompatible pointer type [enabled by default] drivers/rtc/rtc-ds1307.c:1634:2: warning: (near initialization for 'ds1307_driver.id_table') [enabled by default] cc1: some warnings being treated as errors vim +1634 drivers/rtc/rtc-ds1307.c 1618 1619 static int ds1307_remove(struct i2c_client *client) 1620 { 1621 struct ds1307 *ds1307 = i2c_get_clientdata(client); 1622 1623 if (test_and_clear_bit(HAS_NVRAM, &ds1307->flags)) 1624 sysfs_remove_bin_file(&client->dev.kobj, ds1307->nvram); 1625 1626 return 0; 1627 } 1628 1629 static struct i2c_driver ds1307_driver = { 1630 .driver = { 1631 .name = "rtc-ds1307", 1632 .of_match_table = of_match_ptr(ds1307_dt_ids), 1633 }, > 1634 .probe_new = ds1307_probe, 1635 .remove = ds1307_remove, 1636 }; 1637 1638 module_i2c_driver(ds1307_driver); 1639 1640 MODULE_DESCRIPTION("RTC driver for DS1307 and similar chips"); 1641 MODULE_LICENSE("GPL"); --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
> As we match on only the device, not the manufacturer, I've changed your sketch > so that the test maxim line is on a compatible with name ds9999 to make it > unique. Otherwise, we would match to the dallas,ds1307 id type. OK, so I assumed correctly that userspace instantiation wouldn't have worked fully. > If you would prefer that we support separate manufacturers, I can update the > match function so that it attempts a full match first, followed by a stripped > manufacturer match. I'm not certain if we have a need for that at the moment > though, as the current drivers simply match on the device name: I think we *need* that. We can't instantiate my previous example via userspace otherwise. Also, stripping vendor names is what we want to get rid of with this series, no? > This patch also demonstrates a method to obtain the device ID from the new > match system for drivers which would normally have expected this information to > be passed in. Thanks for the testing! Wolfram
Hi Wolfram, On 13/06/16 18:13, Wolfram Sang wrote: > >> As we match on only the device, not the manufacturer, I've changed your sketch >> so that the test maxim line is on a compatible with name ds9999 to make it >> unique. Otherwise, we would match to the dallas,ds1307 id type. > > OK, so I assumed correctly that userspace instantiation wouldn't have > worked fully. > >> If you would prefer that we support separate manufacturers, I can update the >> match function so that it attempts a full match first, followed by a stripped >> manufacturer match. I'm not certain if we have a need for that at the moment >> though, as the current drivers simply match on the device name: > > I think we *need* that. We can't instantiate my previous example via > userspace otherwise. Also, stripping vendor names is what we want to get > rid of with this series, no? Awww <multiple expletives removed> I just re-read this - Has this series been blocked on me without me realising? /me cowers in a corner in shame... I think the aim of this series was to simplify the code structures. Making the vendor names usable, makes sense IMO, but I don't think that was a goal of the original series. I think the series was trying to make an improvement *without* changing functionality / usage. Re-introducing vendor names into the i2c matching would be a change in scope from my view, but if it's required to get this series moving let me know. >> This patch also demonstrates a method to obtain the device ID from the new >> match system for drivers which would normally have expected this information to >> be passed in. > > Thanks for the testing! > > Wolfram >
=======- root@arm:~# echo ds9999 0x68 > /sys/bus/i2c/devices/i2c-2/new_device [ 43.262432] rtc-ds1307 2-0068: I'm a Maxim ... [ 43.268707] rtc-ds1307 2-0068: rtc core: registered ds9999 as rtc0 [ 43.275276] rtc-ds1307 2-0068: 56 bytes nvram [ 43.279920] i2c i2c-2: new_device: Instantiated device ds9999 at 0x68 root@arm:~# cat /sys/class/rtc/rtc0/date 2016-06-12 root@arm:~# cat /sys/class/rtc/rtc0/name ds9999 --- drivers/rtc/rtc-ds1307.c | 60 ++++++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index 821d9c089cdb..d97e8adb866b 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -31,6 +31,7 @@ */ enum ds_type { ds_1307, + maxim_1307, ds_1337, ds_1338, ds_1339, @@ -144,6 +145,10 @@ static struct chip_desc chips[last_ds_type] = { .nvram_offset = 8, .nvram_size = 56, }, + [maxim_1307] = { + .nvram_offset = 8, + .nvram_size = 56, + }, [ds_1337] = { .alarm = 1, }, @@ -173,22 +178,6 @@ static struct chip_desc chips[last_ds_type] = { }, }; -static const struct i2c_device_id ds1307_id[] = { - { "ds1307", ds_1307 }, - { "ds1337", ds_1337 }, - { "ds1338", ds_1338 }, - { "ds1339", ds_1339 }, - { "ds1388", ds_1388 }, - { "ds1340", ds_1340 }, - { "ds3231", ds_3231 }, - { "m41t00", m41t00 }, - { "mcp7940x", mcp794xx }, - { "mcp7941x", mcp794xx }, - { "pt7c4338", ds_1307 }, - { "rx8025", rx_8025 }, - { } -}; -MODULE_DEVICE_TABLE(i2c, ds1307_id); /*----------------------------------------------------------------------*/ @@ -1226,13 +1215,27 @@ static void ds1307_clks_register(struct ds1307 *ds1307) #endif /* CONFIG_COMMON_CLK */ -static int ds1307_probe(struct i2c_client *client, - const struct i2c_device_id *id) +static const struct of_device_id ds1307_dt_ids[] = { + /* We are only matching on the device name, *NOT* the manufacturer name + * I.e. dallas, maxim, are dropped in the search when someone tries to load a + * 'ds1307', and hence first match wins. + * + * We could extend this to do a full match first, followed by a fallback match + * to just the device name. + */ + { .compatible = "dallas,ds1307", .data = (void *)ds_1307 }, + { .compatible = "maxim,ds9999", .data = (void *)maxim_1307 }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ds1307_dt_ids); + +static int ds1307_probe(struct i2c_client *client) { struct ds1307 *ds1307; int err = -ENODEV; int tmp; - struct chip_desc *chip = &chips[id->driver_data]; + const struct of_device_id *idof; + struct chip_desc *chip; struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); bool want_irq = false; bool ds1307_can_wakeup_device = false; @@ -1255,10 +1258,20 @@ static int ds1307_probe(struct i2c_client *client, if (!ds1307) return -ENOMEM; + /* If we've got this far, this shouldn't be able to fail - but check anyway for now */ + idof = i2c_of_match_device(ds1307_dt_ids, client); + if (!idof) { + dev_err(&client->dev, "Probe failed to find an id entry\n"); + return -ENODEV; + } + + /* Now we can set our chip entry */ + chip = &chips[(int)idof->data]; + i2c_set_clientdata(client, ds1307); ds1307->client = client; - ds1307->type = id->driver_data; + ds1307->type = (int) idof->data; if (!pdata && client->dev.of_node) ds1307_trickle_of_init(client, chip); @@ -1435,6 +1448,9 @@ read_rtc: */ tmp = ds1307->regs[DS1307_REG_SECS]; switch (ds1307->type) { + case maxim_1307: + dev_info(&client->dev, "I'm a Maxim ... \n"); + /* fallthrough */ case ds_1307: case m41t00: /* clock halted? turn it on, so clock can tick. */ @@ -1613,10 +1629,10 @@ static int ds1307_remove(struct i2c_client *client) static struct i2c_driver ds1307_driver = { .driver = { .name = "rtc-ds1307", + .of_match_table = of_match_ptr(ds1307_dt_ids), }, - .probe = ds1307_probe, + .probe_new = ds1307_probe, .remove = ds1307_remove, - .id_table = ds1307_id, }; module_i2c_driver(ds1307_driver);