From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754019AbaKDO34 (ORCPT ); Tue, 4 Nov 2014 09:29:56 -0500 Received: from bh-25.webhostbox.net ([208.91.199.152]:41616 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752479AbaKDO3u (ORCPT ); Tue, 4 Nov 2014 09:29:50 -0500 Message-ID: <5458E294.40509@roeck-us.net> Date: Tue, 04 Nov 2014 06:28:36 -0800 From: Guenter Roeck User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Neelesh Gupta , jdelvare@suse.de CC: benh@au1.ibm.com, linux-kernel@vger.kernel.org, michaele@au1.ibm.com, lm-sensors@lm-sensors.org Subject: Re: [PATCH] hwmon: (ibmpowernv) Use platform 'id_table' to probe the device References: <20141104072927.4307.67881.stgit@localhost.localdomain> In-Reply-To: <20141104072927.4307.67881.stgit@localhost.localdomain> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-CTCH-PVer: 0000001 X-CTCH-Spam: Unknown X-CTCH-VOD: Unknown X-CTCH-Flags: 0 X-CTCH-RefID: str=0001.0A02020A.5458E2DE.002C,ss=1,re=0.001,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0 X-CTCH-Score: 0.001 X-CTCH-ScoreCust: 0.000 X-CTCH-Rules: C_4847, X-CTCH-SenderID: linux@roeck-us.net X-CTCH-SenderID-Flags: 0 X-CTCH-SenderID-TotalMessages: 3 X-CTCH-SenderID-TotalSpam: 0 X-CTCH-SenderID-TotalSuspected: 0 X-CTCH-SenderID-TotalConfirmed: 0 X-CTCH-SenderID-TotalBulk: 0 X-CTCH-SenderID-TotalVirus: 0 X-CTCH-SenderID-TotalRecipients: 0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: mailgid no entry from get_relayhosts_entry X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/03/2014 11:30 PM, Neelesh Gupta wrote: > The current driver probe() function assumes the sensor device to be > alwary present and gets executed every time if the driver is loaded, > but the appropriate hardware could not be present. > > So, move the platform device creation as part of platform init code > and use the 'id_table' to check if the device present or not. > > Signed-off-by: Neelesh Gupta > --- > arch/powerpc/platforms/powernv/opal-sensor.c | 20 ++++++++ > drivers/hwmon/ibmpowernv.c | 67 +++++++------------------- > 2 files changed, 39 insertions(+), 48 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/opal-sensor.c b/arch/powerpc/platforms/powernv/opal-sensor.c > index 10271ad..215c418 100644 > --- a/arch/powerpc/platforms/powernv/opal-sensor.c > +++ b/arch/powerpc/platforms/powernv/opal-sensor.c > @@ -20,7 +20,9 @@ > > #include > #include > +#include > #include > +#include > > static DEFINE_MUTEX(opal_sensor_mutex); > > @@ -64,3 +66,21 @@ out: > return ret; > } > EXPORT_SYMBOL_GPL(opal_get_sensor_data); > + > +static __init int opal_sensor_init(void) > +{ > + struct platform_device *pdev; > + struct device_node *sensor; > + > + sensor = of_find_node_by_path("/ibm,opal/sensors"); > + if (sensor) { > + pdev = of_platform_device_create(sensor, "opal-sensor", NULL); > + of_node_put(sensor); > + } else { > + pr_info("Opal node 'sensors' not found\n"); Hi Neelesh, this is either an error and should be reported with pr_err, or it is a valid condition and should be reported with pr_debug or not at all. Which one is it ? Also, can you put this code into a 'normal' error handler instead of using if/else ? if (!sensor) { pr_err("Opal node 'sensors' not found\n"); return -ENODEV; } pdev = of_platform_device_create(sensor, "opal-sensor", NULL); ... This would make the code flow easier to understand. Thanks, Guenter > + return -ENODEV; > + } > + > + return PTR_ERR_OR_ZERO(pdev); > +} > +machine_subsys_initcall(powernv, opal_sensor_init); > diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c > index 6a30eee..c7577b8 100644 > --- a/drivers/hwmon/ibmpowernv.c > +++ b/drivers/hwmon/ibmpowernv.c > @@ -74,9 +74,6 @@ struct platform_data { > u32 sensors_count; /* Total count of sensors from each group */ > }; > > -/* Platform device representing all the ibmpowernv sensors */ > -static struct platform_device *pdevice; > - > static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr, > char *buf) > { > @@ -99,7 +96,7 @@ static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr, > return sprintf(buf, "%u\n", x); > } > > -static int __init get_sensor_index_attr(const char *name, u32 *index, > +static int get_sensor_index_attr(const char *name, u32 *index, > char *attr) > { > char *hash_pos = strchr(name, '#'); > @@ -136,7 +133,7 @@ static int __init get_sensor_index_attr(const char *name, u32 *index, > * which need to be mapped as fan2_input, temp1_max respectively before > * populating them inside hwmon device class. > */ > -static int __init create_hwmon_attr_name(struct device *dev, enum sensors type, > +static int create_hwmon_attr_name(struct device *dev, enum sensors type, > const char *node_name, > char *hwmon_attr_name) > { > @@ -172,7 +169,7 @@ static int __init create_hwmon_attr_name(struct device *dev, enum sensors type, > return 0; > } > > -static int __init populate_attr_groups(struct platform_device *pdev) > +static int populate_attr_groups(struct platform_device *pdev) > { > struct platform_data *pdata = platform_get_drvdata(pdev); > const struct attribute_group **pgroups = pdata->attr_groups; > @@ -180,11 +177,6 @@ static int __init populate_attr_groups(struct platform_device *pdev) > enum sensors type; > > opal = of_find_node_by_path("/ibm,opal/sensors"); > - if (!opal) { > - dev_dbg(&pdev->dev, "Opal node 'sensors' not found\n"); > - return -ENODEV; > - } > - > for_each_child_of_node(opal, np) { > if (np->name == NULL) > continue; > @@ -221,7 +213,7 @@ static int __init populate_attr_groups(struct platform_device *pdev) > * to the name required by the higher 'hwmon' driver like fan1_input, temp1_max > * etc.. > */ > -static int __init create_device_attrs(struct platform_device *pdev) > +static int create_device_attrs(struct platform_device *pdev) > { > struct platform_data *pdata = platform_get_drvdata(pdev); > const struct attribute_group **pgroups = pdata->attr_groups; > @@ -280,7 +272,7 @@ exit_put_node: > return err; > } > > -static int __init ibmpowernv_probe(struct platform_device *pdev) > +static int ibmpowernv_probe(struct platform_device *pdev) > { > struct platform_data *pdata; > struct device *hwmon_dev; > @@ -309,52 +301,31 @@ static int __init ibmpowernv_probe(struct platform_device *pdev) > return PTR_ERR_OR_ZERO(hwmon_dev); > } > > +static const struct platform_device_id opal_sensor_driver_ids[] = { > + { > + .name = "opal-sensor", > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(platform, opal_sensor_driver_ids); > + > static struct platform_driver ibmpowernv_driver = { > - .driver = { > - .owner = THIS_MODULE, > - .name = DRVNAME, > + .probe = ibmpowernv_probe, > + .id_table = opal_sensor_driver_ids, > + .driver = { > + .owner = THIS_MODULE, > + .name = DRVNAME, > }, > }; > > static int __init ibmpowernv_init(void) > { > - int err; > - > - pdevice = platform_device_alloc(DRVNAME, 0); > - if (!pdevice) { > - pr_err("Device allocation failed\n"); > - err = -ENOMEM; > - goto exit; > - } > - > - err = platform_device_add(pdevice); > - if (err) { > - pr_err("Device addition failed (%d)\n", err); > - goto exit_device_put; > - } > - > - err = platform_driver_probe(&ibmpowernv_driver, ibmpowernv_probe); > - if (err) { > - if (err != -ENODEV) > - pr_err("Platform driver probe failed (%d)\n", err); > - > - goto exit_device_del; > - } > - > - return 0; > - > -exit_device_del: > - platform_device_del(pdevice); > -exit_device_put: > - platform_device_put(pdevice); > -exit: > - return err; > + return platform_driver_register(&ibmpowernv_driver); > } > > static void __exit ibmpowernv_exit(void) > { > platform_driver_unregister(&ibmpowernv_driver); > - platform_device_unregister(pdevice); > } > > MODULE_AUTHOR("Neelesh Gupta "); > >