[PATCH v2 2/2] i2c-i801: Don't depend on other kernel driver config options
Don't let other driver config options influence us, as it makes the
code more complex and fragile for a small benefit. There's nothing
wrong with instantiating I2C devices even if they don't have a driver.
And we're talking about 835 extra bytes in the binary on x86-64,
that's hardly worth arguing about.
Signed-off-by: Jean Delvare
Cc: David Woodhouse
Cc: Hans de Goede
---
Changes since v1:
* Make the optional code depend on CONFIG_DMI too.
* Exclude i801_probe_optional_slaves() completely if the optional code
is missing, result is the same and this makes the code clearer.
drivers/i2c/busses/i2c-i801.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
--- linux-2.6.39-rc7.orig/drivers/i2c/busses/i2c-i801.c 2011-05-15
21:31:42.0 +0200
+++ linux-2.6.39-rc7/drivers/i2c/busses/i2c-i801.c 2011-05-15
22:07:58.0 +0200
@@ -638,7 +638,7 @@ static const struct pci_device_id i801_i
MODULE_DEVICE_TABLE(pci, i801_ids);
-#if defined CONFIG_INPUT_APANEL || defined CONFIG_INPUT_APANEL_MODULE
+#if defined CONFIG_X86 && defined CONFIG_DMI
static unsigned char apanel_addr;
/* Scan the system ROM for the signature "FJKEYINF" */
@@ -668,11 +668,7 @@ static void __init input_apanel_init(voi
}
iounmap(bios);
}
-#else
-static void __init input_apanel_init(void) {}
-#endif
-#if defined CONFIG_SENSORS_FSCHMD || defined CONFIG_SENSORS_FSCHMD_MODULE
struct dmi_onboard_device_info {
const char *name;
u8 type;
@@ -738,7 +734,6 @@ static void __devinit dmi_check_onboard_
dmi_check_onboard_device(type, name, adap);
}
}
-#endif
/* Register optional slaves */
static void __devinit i801_probe_optional_slaves(struct i801_priv *priv)
@@ -747,7 +742,6 @@ static void __devinit i801_probe_optiona
if (priv->features & FEATURE_IDF)
return;
-#if defined CONFIG_INPUT_APANEL || defined CONFIG_INPUT_APANEL_MODULE
if (apanel_addr) {
struct i2c_board_info info;
@@ -756,12 +750,14 @@ static void __devinit i801_probe_optiona
strlcpy(info.type, "fujitsu_apanel", I2C_NAME_SIZE);
i2c_new_device(&priv->adapter, &info);
}
-#endif
-#if defined CONFIG_SENSORS_FSCHMD || defined CONFIG_SENSORS_FSCHMD_MODULE
+
if (dmi_name_in_vendors("FUJITSU"))
dmi_walk(dmi_check_onboard_devices, &priv->adapter);
-#endif
}
+#else
+static void __init input_apanel_init(void) {}
+static void __devinit i801_probe_optional_slaves(struct i801_priv *priv) {}
+#endif /* CONFIG_X86 && CONFIG_DMI */
static int __devinit i801_probe(struct pci_dev *dev,
const struct pci_device_id *id)
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] i2c-i801: Check for vendor Fujitsu before probing for apanel
Scanning the BIOS memory for the apanel information is costly, so avoid
doing it on non-Fujitsu machines.
Signed-off-by: Jean Delvare
---
drivers/i2c/busses/i2c-i801.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-2.6.39-rc7.orig/drivers/i2c/busses/i2c-i801.c 2011-05-15
21:13:06.0 +0200
+++ linux-2.6.39-rc7/drivers/i2c/busses/i2c-i801.c 2011-05-15
21:31:42.0 +0200
@@ -931,7 +931,8 @@ static struct pci_driver i801_driver = {
static int __init i2c_i801_init(void)
{
- input_apanel_init();
+ if (dmi_name_in_vendors("FUJITSU"))
+ input_apanel_init();
return pci_register_driver(&i801_driver);
}
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-i801: Don't depend on other kernel driver config options
Hi David, On Fri, 13 May 2011 11:23:06 +0100, Woodhouse, David wrote: > On Fri, 2011-05-13 at 11:01 +0100, Jean Delvare wrote: > > Don't let other driver config options influence us, as it makes the > > code more complex and fragile for a small benefit. There's nothing > > wrong with instantiating I2C devices even if they don't have a driver. > > And we're talking about 835 extra bytes in the binary on x86-64, > > that's hardly worth arguing about. > > Looks sane to me; thanks. Should it be CONFIG_DMI instead of (or in > addition to) CONFIG_X86? Instead of, no. IA64 has DMI but doesn't want this code. In addition to, yes, that would make sense. I expected the extra code to transparently vanish when CONFIG_DMI isn't set, but gcc is apparently not smart enough for this, so it could use some help. BTW, it may be desirable to make the apanel part depend on DMI, even if DMI is technically not needed to find the slave address. Scanning the BIOS memory for an arbitrary string is costly (a minimum of 4096 calls to check_signature(), which in turn calls readb() at least once, in the negative case.) Checking for the DMI vendor first should be way less costly for the negative case. I'll submit a new patch set later today, thanks for the suggestions. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pwm: Add __weak attributed functions for pwm operations
On Sun, May 15, 2011 at 01:00:31AM -0700, Dmitry Torokhov wrote: > On Fri, May 13, 2011 at 06:13:21PM +0530, Mohan Pallaka wrote: > > For chip drivers that support both pwm and non-pwm modes > > would encounter compilation errors if the platform doesn't > > have support for pwm though the chip is programmed to work > > in non-pwm mode. Add __weak attributed pwm functions to avoid > > compilation issues in these scenarios. > > > > Russell, > > You seem to have authored pwm.h, do you have any objections to this > change? This seems to be a recipe for an oops. Have a look at how linux/regulator/consumer.h deals with this kind of problem, and notice that we have CONFIG_HAVE_PWM to indicate whether this interface is supported or not. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pwm: Add __weak attributed functions for pwm operations
On Fri, May 13, 2011 at 06:13:21PM +0530, Mohan Pallaka wrote: > For chip drivers that support both pwm and non-pwm modes > would encounter compilation errors if the platform doesn't > have support for pwm though the chip is programmed to work > in non-pwm mode. Add __weak attributed pwm functions to avoid > compilation issues in these scenarios. > Russell, You seem to have authored pwm.h, do you have any objections to this change? Thanks! > Change-Id: Ia507bf659d4d67d71f135012e7d919aca6c45c6c > Signed-off-by: Mohan Pallaka > --- > include/linux/pwm.h | 12 +++- > 1 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > index 7c77575..e0c8c3f 100644 > --- a/include/linux/pwm.h > +++ b/include/linux/pwm.h > @@ -3,29 +3,31 @@ > > struct pwm_device; > > +/* Add __weak functions to support PWM */ > + > /* > * pwm_request - request a PWM device > */ > -struct pwm_device *pwm_request(int pwm_id, const char *label); > +struct pwm_device __weak *pwm_request(int pwm_id, const char *label); > > /* > * pwm_free - free a PWM device > */ > -void pwm_free(struct pwm_device *pwm); > +void __weak pwm_free(struct pwm_device *pwm); > > /* > * pwm_config - change a PWM device configuration > */ > -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); > +int __weak pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); > > /* > * pwm_enable - start a PWM output toggling > */ > -int pwm_enable(struct pwm_device *pwm); > +int __weak pwm_enable(struct pwm_device *pwm); > > /* > * pwm_disable - stop a PWM output toggling > */ > -void pwm_disable(struct pwm_device *pwm); > +void __weak pwm_disable(struct pwm_device *pwm); > > #endif /* __LINUX_PWM_H */ > > -- > Sent by a consultant of the Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pwm: Add __weak attributed functions for pwm operations
On Fri, May 13, 2011 at 06:13:21PM +0530, Mohan Pallaka wrote: > For chip drivers that support both pwm and non-pwm modes > would encounter compilation errors if the platform doesn't > have support for pwm though the chip is programmed to work > in non-pwm mode. Add __weak attributed pwm functions to avoid > compilation issues in these scenarios. > Russell, You seem to have authored pwm.h, do you have any objections to this change? Thanks! > Change-Id: Ia507bf659d4d67d71f135012e7d919aca6c45c6c > Signed-off-by: Mohan Pallaka > --- > include/linux/pwm.h | 12 +++- > 1 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > index 7c77575..e0c8c3f 100644 > --- a/include/linux/pwm.h > +++ b/include/linux/pwm.h > @@ -3,29 +3,31 @@ > > struct pwm_device; > > +/* Add __weak functions to support PWM */ > + > /* > * pwm_request - request a PWM device > */ > -struct pwm_device *pwm_request(int pwm_id, const char *label); > +struct pwm_device __weak *pwm_request(int pwm_id, const char *label); > > /* > * pwm_free - free a PWM device > */ > -void pwm_free(struct pwm_device *pwm); > +void __weak pwm_free(struct pwm_device *pwm); > > /* > * pwm_config - change a PWM device configuration > */ > -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); > +int __weak pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); > > /* > * pwm_enable - start a PWM output toggling > */ > -int pwm_enable(struct pwm_device *pwm); > +int __weak pwm_enable(struct pwm_device *pwm); > > /* > * pwm_disable - stop a PWM output toggling > */ > -void pwm_disable(struct pwm_device *pwm); > +void __weak pwm_disable(struct pwm_device *pwm); > > #endif /* __LINUX_PWM_H */ > > -- > Sent by a consultant of the Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
