From: Wolfgang Grandegger <wg@grandegger•com>
To: Jean Delvare <khali@linux-fr•org>
Cc: Linuxppc-dev@ozlabs•org
Subject: Re: [RFC] I2C: fsl-i2c: make device probing configurable via FDT
Date: Wed, 16 Jul 2008 15:03:19 +0200 [thread overview]
Message-ID: <487DF197.3090305@grandegger.com> (raw)
In-Reply-To: <20080716140511.7a269709@hyperion.delvare>
Jean Delvare wrote:
> Hi Wolfgang,
>
> On Wed, 16 Jul 2008 12:47:25 +0200, Wolfgang Grandegger wrote:
>> Currently, the I2C buses are probed for HWMON I2C devices, which might
>> not be acceptable in same cases. This patch makes device probing
>> configurable through the property "probe" of the FDT I2C device node:
>>
>> i2c@3000 {
>> ...
>> compatible = "fsl-i2c";
>> probe;
>> ...
>> };
>>
>> Assuming that systems using fsl-i2c should have proper platform data,
>> probing is disabled by default. Unfortunately, that's not the case and
>> various DTS files would require to be updated.
>>
>> There have already been some discussion on this subject with Jean
>> under the following thread:
>>
>> http://ozlabs.org/pipermail/linuxppc-dev/2008-July/060008.html
>>
>> Please comment.
>>
>> Thanks,
>>
>> Wolfgang.
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger•com>
>> ---
>> arch/powerpc/sysdev/fsl_soc.c | 4 ++++
>> drivers/i2c/busses/i2c-mpc.c | 17 ++++++++++++++---
>> include/linux/fsl_devices.h | 1 +
>> 3 files changed, 19 insertions(+), 3 deletions(-)
>>
>> Index: linux-2.6-denx/drivers/i2c/busses/i2c-mpc.c
>> ===================================================================
>> --- linux-2.6-denx.orig/drivers/i2c/busses/i2c-mpc.c
>> +++ linux-2.6-denx/drivers/i2c/busses/i2c-mpc.c
>> @@ -306,13 +306,21 @@ static const struct i2c_algorithm mpc_al
>> .functionality = mpc_functionality,
>> };
>>
>> -static struct i2c_adapter mpc_ops = {
>> +static struct i2c_adapter mpc_probe_ops = {
>> .owner = THIS_MODULE,
>> .name = "MPC adapter",
>> .id = I2C_HW_MPC107,
>> .algo = &mpc_algo,
>> .class = I2C_CLASS_HWMON,
>> - .timeout = 1,
>> + .timeout = 1,
>
> Your patch is adding undesirable leading white space.
>
>> +};
>> +
>> +static struct i2c_adapter mpc_ops = {
>> + .owner = THIS_MODULE,
>> + .name = "MPC adapter",
>> + .id = I2C_HW_MPC107,
>> + .algo = &mpc_algo,
>> + .timeout = 1,
>> };
>
> I don't think you need 2 separate structures. Just set the class field
> at runtime (see below.)
>
>>
>> static int fsl_i2c_probe(struct platform_device *pdev)
>> @@ -354,7 +362,10 @@ static int fsl_i2c_probe(struct platform
>> mpc_i2c_setclock(i2c);
>> platform_set_drvdata(pdev, i2c);
>>
>> - i2c->adap = mpc_ops;
>> + (i2c->flags & FSL_I2C_DEV_PROBE)
>
> Missing if? :)
Oops, I obviously forgot the quilt refresh before sending.
>
>> + i2c->adap = mpc_probe_ops;
>> + else
>> + i2c->adap = mpc_ops;
>
> Or just:
>
> i2c->adap = mpc_ops;
> if (i2c->flags & FSL_I2C_DEV_PROBE)
> i2c->adap.flags = I2C_CLASS_HWMON;
Right, mpc_ops is not a pointer, as I realize now.
>> i2c->adap.nr = pdev->id;
>> i2c_set_adapdata(&i2c->adap, i2c);
>> i2c->adap.dev.parent = &pdev->dev;
>> Index: linux-2.6-denx/arch/powerpc/sysdev/fsl_soc.c
>> ===================================================================
>> --- linux-2.6-denx.orig/arch/powerpc/sysdev/fsl_soc.c
>> +++ linux-2.6-denx/arch/powerpc/sysdev/fsl_soc.c
>> @@ -518,6 +518,10 @@ static int __init fsl_i2c_of_init(void)
>> if (flags)
>> i2c_data.device_flags |= FSL_I2C_DEV_CLOCK_5200;
>>
>> + flags = of_get_property(np, "probe", NULL);
>> + if (flags)
>> + i2c_data.device_flags |= FSL_I2C_DEV_PROBE;
>> +
>> ret =
>> platform_device_add_data(i2c_dev, &i2c_data,
>> sizeof(struct
>> Index: linux-2.6-denx/include/linux/fsl_devices.h
>> ===================================================================
>> --- linux-2.6-denx.orig/include/linux/fsl_devices.h
>> +++ linux-2.6-denx/include/linux/fsl_devices.h
>> @@ -82,6 +82,7 @@ struct fsl_i2c_platform_data {
>> /* Flags related to I2C device features */
>> #define FSL_I2C_DEV_SEPARATE_DFSRR 0x00000001
>> #define FSL_I2C_DEV_CLOCK_5200 0x00000002
>> +#define FSL_I2C_DEV_PROBE 0x00000004
>>
>> enum fsl_usb2_operating_modes {
>> FSL_USB2_MPH_HOST,
>
> As a side note: in general it is not a good idea to use a template for
> struct i2c_adapter. This structure is relatively large compared to the
> few fields you need to set. If you insist on having a template, it
> should at least be marked either const or __initdata.
Wolfgang.
next prev parent reply other threads:[~2008-07-16 13:03 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-16 10:47 [RFC] I2C: fsl-i2c: make device probing configurable via FDT Wolfgang Grandegger
2008-07-16 12:05 ` Jean Delvare
2008-07-16 13:03 ` Wolfgang Grandegger [this message]
2008-07-16 12:47 ` Jon Smirl
2008-07-16 13:09 ` Wolfgang Grandegger
2008-07-16 14:11 ` Jon Smirl
2008-07-16 14:24 ` Jon Smirl
2008-07-16 14:30 ` Grant Likely
2008-07-16 14:42 ` Jon Smirl
2008-07-16 15:01 ` Timur Tabi
2008-07-16 14:24 ` Grant Likely
2008-07-16 14:48 ` Jochen Friedrich
2008-07-16 20:20 ` Wolfgang Grandegger
2008-07-17 10:20 ` Is there relationship between address translation enabled and PLB timeout error? Evangelion
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=487DF197.3090305@grandegger.com \
--to=wg@grandegger$(echo .)com \
--cc=Linuxppc-dev@ozlabs$(echo .)org \
--cc=khali@linux-fr$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox