Messages in this thread |  | | Date | Fri, 10 Feb 2012 11:07:55 -0500 | From | Vivien Didelot <> | Subject | Re: [PATCH v5 4/5] hwmon: add MAX197 support |
| |
Hi,
On Mon, 6 Feb 2012 12:46:04 -0800, Guenter Roeck <guenter.roeck@ericsson.com> wrote:
> On Mon, 2012-02-06 at 15:15 -0500, Vivien Didelot wrote: > [ ... ] > > > > BTW, about the TS-5500 ADC part, is a platform ts5500_adc.c file the > > better solution, or should the device be declared in the ts5500.c > > platform code? > > > I would suggest to declare it in the ts5500.c platform code. That > seems to be the common approach as far as I can see. > > platform_add_devices() works pretty well for this. It saves you from > having to call platform_device_register() for each device separately. > Obviously that only works if all devices are declared in a single > file.
As the LED is registered using the leds_class, I think platform_add_devices() couldn't be used here.
Lots of platform codes don't check the returned value of platform_add_devices(). Should we care about a LED or ADC registration failure (is the following snippet OK?)?
static int __init ts5500_init(void) { [...] pdev = platform_device_register_simple("ts5500", -1, NULL, 0); if (IS_ERR(pdev)) { ret = PTR_ERR(pdev); goto release_mem; } platform_set_drvdata(pdev, ts5500); ret = sysfs_create_group(&pdev->dev.kobj, &ts5500_attr_group); if (ret) goto release_pdev; led_classdev_register(&pdev->dev, &ts5500_led_cdev); if (ts5500->adc) { ts5500_adc_pdev.dev.parent = &pdev->dev; platform_device_register(&ts5500_adc_pdev); } return 0; release_pdev: platform_device_unregister(pdev); release_mem: kfree(ts5500); return ret; } device_initcall(ts5500_init);
Thanks,
-- Vivien Didelot Savoir-faire Linux Inc. Tel: (514) 276-5468 #149
|  |