* cannot ret error from probe - switch tuner to I2C driver model
@ 2013-10-14 0:10 Antti Palosaari
2013-10-14 2:31 ` Antti Palosaari
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2013-10-14 0:10 UTC (permalink / raw)
To: linux-media; +Cc: Antti Palosaari
kernel: usb 1-2: rtl2832u_tuner_attach:
kernel: e4000 5-0064: e4000_probe:
kernel: usb 1-2: rtl2832u_tuner_attach: client ptr ffff88030a849000
See attached patch.
Is there any way to return error to caller?
Abuse platform data ptr from struct i2c_board_info and call i2c_unregister_device() ?
regards
Antti
---
drivers/media/tuners/e4000.c | 31 +++++++++++++++++++++++++++++++
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 18 ++++++++++++++++--
2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 54e2d8a..f4e0567 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -442,6 +442,37 @@ err:
}
EXPORT_SYMBOL(e4000_attach);
+static int e4000_probe(struct i2c_client *client, const struct i2c_device_id *did)
+{
+ dev_info(&client->dev, "%s:\n", __func__);
+ return -ENODEV;
+}
+
+static int e4000_remove(struct i2c_client *client)
+{
+ dev_info(&client->dev, "%s:\n", __func__);
+ return 0;
+}
+
+static const struct i2c_device_id e4000_id[] = {
+ {"e4000", 0},
+ {}
+};
+
+MODULE_DEVICE_TABLE(i2c, e4000_id);
+
+static struct i2c_driver e4000_driver = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "e4000",
+ },
+ .probe = e4000_probe,
+ .remove = e4000_remove,
+ .id_table = e4000_id,
+};
+
+module_i2c_driver(e4000_driver);
+
MODULE_DESCRIPTION("Elonics E4000 silicon tuner driver");
MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
MODULE_LICENSE("GPL");
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index defc491..fbbe867 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -898,8 +898,22 @@ static int rtl2832u_tuner_attach(struct dvb_usb_adapter *adap)
adap->fe[0]->ops.tuner_ops.get_rf_strength;
return 0;
case TUNER_RTL2832_E4000:
- fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
- &rtl2832u_e4000_config);
+// fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
+// &rtl2832u_e4000_config);
+ {
+ static const struct i2c_board_info info = {
+ .type = "e4000",
+ .addr = 0x64,
+ };
+ struct i2c_client *client;
+
+ fe = NULL;
+ client = i2c_new_device(&d->i2c_adap, &info);
+ if (IS_ERR_OR_NULL(client))
+ dev_err(&d->udev->dev, "e4000 probe failed\n");
+
+ dev_dbg(&d->udev->dev, "%s: client ptr %p\n", __func__, client);
+ }
break;
case TUNER_RTL2832_FC2580:
fe = dvb_attach(fc2580_attach, adap->fe[0], &d->i2c_adap,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: cannot ret error from probe - switch tuner to I2C driver model
2013-10-14 0:10 cannot ret error from probe - switch tuner to I2C driver model Antti Palosaari
@ 2013-10-14 2:31 ` Antti Palosaari
2013-10-15 15:13 ` Antti Palosaari
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2013-10-14 2:31 UTC (permalink / raw)
To: linux-media
On 14.10.2013 03:10, Antti Palosaari wrote:
> kernel: usb 1-2: rtl2832u_tuner_attach:
> kernel: e4000 5-0064: e4000_probe:
> kernel: usb 1-2: rtl2832u_tuner_attach: client ptr ffff88030a849000
>
> See attached patch.
>
> Is there any way to return error to caller?
>
> Abuse platform data ptr from struct i2c_board_info and call i2c_unregister_device() ?
Answer to myself: best option seems to be check i2c_get_clientdata()
pointer after i2c_new_device().
client = i2c_new_device(&d->i2c_adap, &info);
if (client)
if (i2c_get_clientdata(client) == NULL)
// OOPS, I2C probe fails
That is because it is set NULL in error case by really_probe() in
drivers/base/dd.c. Error status is also cleared there with comment:
/*
* Ignore errors returned by ->probe so that the next driver can try
* its luck.
*/
That is told in I2C documentation too:
Note that starting with kernel 2.6.34, you don't have to set the `data'
field
to NULL in remove() or if probe() failed anymore. The i2c-core does this
automatically on these occasions. Those are also the only times the core
will
touch this field.
But maybe the comment for actual function, i2c_new_device, is still a
bit misleading as it says NULL is returned for the error. All the other
errors yes, but not for the I2C .probe() as it is reseted by device core.
* This returns the new i2c client, which may be saved for later use with
* i2c_unregister_device(); or NULL to indicate an error.
*/
struct i2c_client *
i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
regards
Antti
>
> regards
> Antti
>
> ---
> drivers/media/tuners/e4000.c | 31 +++++++++++++++++++++++++++++++
> drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 18 ++++++++++++++++--
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
> index 54e2d8a..f4e0567 100644
> --- a/drivers/media/tuners/e4000.c
> +++ b/drivers/media/tuners/e4000.c
> @@ -442,6 +442,37 @@ err:
> }
> EXPORT_SYMBOL(e4000_attach);
>
> +static int e4000_probe(struct i2c_client *client, const struct i2c_device_id *did)
> +{
> + dev_info(&client->dev, "%s:\n", __func__);
> + return -ENODEV;
> +}
> +
> +static int e4000_remove(struct i2c_client *client)
> +{
> + dev_info(&client->dev, "%s:\n", __func__);
> + return 0;
> +}
> +
> +static const struct i2c_device_id e4000_id[] = {
> + {"e4000", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, e4000_id);
> +
> +static struct i2c_driver e4000_driver = {
> + .driver = {
> + .owner = THIS_MODULE,
> + .name = "e4000",
> + },
> + .probe = e4000_probe,
> + .remove = e4000_remove,
> + .id_table = e4000_id,
> +};
> +
> +module_i2c_driver(e4000_driver);
> +
> MODULE_DESCRIPTION("Elonics E4000 silicon tuner driver");
> MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> index defc491..fbbe867 100644
> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> @@ -898,8 +898,22 @@ static int rtl2832u_tuner_attach(struct dvb_usb_adapter *adap)
> adap->fe[0]->ops.tuner_ops.get_rf_strength;
> return 0;
> case TUNER_RTL2832_E4000:
> - fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
> - &rtl2832u_e4000_config);
> +// fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
> +// &rtl2832u_e4000_config);
> + {
> + static const struct i2c_board_info info = {
> + .type = "e4000",
> + .addr = 0x64,
> + };
> + struct i2c_client *client;
> +
> + fe = NULL;
> + client = i2c_new_device(&d->i2c_adap, &info);
> + if (IS_ERR_OR_NULL(client))
> + dev_err(&d->udev->dev, "e4000 probe failed\n");
> +
> + dev_dbg(&d->udev->dev, "%s: client ptr %p\n", __func__, client);
> + }
> break;
> case TUNER_RTL2832_FC2580:
> fe = dvb_attach(fc2580_attach, adap->fe[0], &d->i2c_adap,
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cannot ret error from probe - switch tuner to I2C driver model
2013-10-14 2:31 ` Antti Palosaari
@ 2013-10-15 15:13 ` Antti Palosaari
2013-10-15 17:11 ` Jean Delvare
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2013-10-15 15:13 UTC (permalink / raw)
To: linux-media, Jean Delvare
Jean
could you look that and comment how I should implement it properly.
I saw you also added i2c_new_probed_device() [1] that these TV drivers
could be ported properly I2C model.
[1] http://lists.lm-sensors.org/pipermail/i2c/2007-March/000990.html
I am pretty sure I have looked and tested all the I2C pieces quite
carefully. There is .detect() callback which looks just what I need, but
unfortunately it is nor called in case of i2c_new_probed_device() and
i2c_new_device().
On 14.10.2013 05:31, Antti Palosaari wrote:
> On 14.10.2013 03:10, Antti Palosaari wrote:
>> kernel: usb 1-2: rtl2832u_tuner_attach:
>> kernel: e4000 5-0064: e4000_probe:
>> kernel: usb 1-2: rtl2832u_tuner_attach: client ptr ffff88030a849000
>>
>> See attached patch.
>>
>> Is there any way to return error to caller?
>>
>> Abuse platform data ptr from struct i2c_board_info and call
>> i2c_unregister_device() ?
>
> Answer to myself: best option seems to be check i2c_get_clientdata()
> pointer after i2c_new_device().
>
> client = i2c_new_device(&d->i2c_adap, &info);
> if (client)
> if (i2c_get_clientdata(client) == NULL)
> // OOPS, I2C probe fails
>
> That is because it is set NULL in error case by really_probe() in
> drivers/base/dd.c. Error status is also cleared there with comment:
> /*
> * Ignore errors returned by ->probe so that the next driver can try
> * its luck.
> */
>
> That is told in I2C documentation too:
> Note that starting with kernel 2.6.34, you don't have to set the `data'
> field
> to NULL in remove() or if probe() failed anymore. The i2c-core does this
> automatically on these occasions. Those are also the only times the core
> will
> touch this field.
>
>
>
> But maybe the comment for actual function, i2c_new_device, is still a
> bit misleading as it says NULL is returned for the error. All the other
> errors yes, but not for the I2C .probe() as it is reseted by device core.
>
> * This returns the new i2c client, which may be saved for later use with
> * i2c_unregister_device(); or NULL to indicate an error.
> */
> struct i2c_client *
> i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
>
>
> regards
> Antti
>
>
>>
>> regards
>> Antti
>>
>> ---
>> drivers/media/tuners/e4000.c | 31
>> +++++++++++++++++++++++++++++++
>> drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 18 ++++++++++++++++--
>> 2 files changed, 47 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
>> index 54e2d8a..f4e0567 100644
>> --- a/drivers/media/tuners/e4000.c
>> +++ b/drivers/media/tuners/e4000.c
>> @@ -442,6 +442,37 @@ err:
>> }
>> EXPORT_SYMBOL(e4000_attach);
>>
>> +static int e4000_probe(struct i2c_client *client, const struct
>> i2c_device_id *did)
>> +{
>> + dev_info(&client->dev, "%s:\n", __func__);
>> + return -ENODEV;
>> +}
>> +
>> +static int e4000_remove(struct i2c_client *client)
>> +{
>> + dev_info(&client->dev, "%s:\n", __func__);
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id e4000_id[] = {
>> + {"e4000", 0},
>> + {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, e4000_id);
>> +
>> +static struct i2c_driver e4000_driver = {
>> + .driver = {
>> + .owner = THIS_MODULE,
>> + .name = "e4000",
>> + },
>> + .probe = e4000_probe,
>> + .remove = e4000_remove,
>> + .id_table = e4000_id,
>> +};
>> +
>> +module_i2c_driver(e4000_driver);
>> +
>> MODULE_DESCRIPTION("Elonics E4000 silicon tuner driver");
>> MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
>> b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
>> index defc491..fbbe867 100644
>> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
>> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
>> @@ -898,8 +898,22 @@ static int rtl2832u_tuner_attach(struct
>> dvb_usb_adapter *adap)
>> adap->fe[0]->ops.tuner_ops.get_rf_strength;
>> return 0;
>> case TUNER_RTL2832_E4000:
>> - fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
>> - &rtl2832u_e4000_config);
>> +// fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
>> +// &rtl2832u_e4000_config);
>> + {
>> + static const struct i2c_board_info info = {
>> + .type = "e4000",
>> + .addr = 0x64,
>> + };
>> + struct i2c_client *client;
>> +
>> + fe = NULL;
>> + client = i2c_new_device(&d->i2c_adap, &info);
>> + if (IS_ERR_OR_NULL(client))
>> + dev_err(&d->udev->dev, "e4000 probe failed\n");
>> +
>> + dev_dbg(&d->udev->dev, "%s: client ptr %p\n", __func__,
>> client);
>> + }
>> break;
>> case TUNER_RTL2832_FC2580:
>> fe = dvb_attach(fc2580_attach, adap->fe[0], &d->i2c_adap,
>>
>
>
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cannot ret error from probe - switch tuner to I2C driver model
2013-10-15 15:13 ` Antti Palosaari
@ 2013-10-15 17:11 ` Jean Delvare
0 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2013-10-15 17:11 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media
Hi Antti,
The driver's .probe() after i2c_new_device() is simply not supposed to
fail. You should only use i2c_new_device() if you are 100% sure that
there is a device of the given type at the given address. Checking
i2c_get_clientdata() is an ugly trick that you should no longer need to
use.
i2c_driver.detect() is used for fully auto-detectable, standalone
devices. Mostly hardware monitoring chips fall into this category. You
should never use that mechanism for video chips, and as a matter of
fact, I2C_CLASS_* flags for analog and digital TV were killed years
ago. To make it clearer: for everything TV/video related, only .probe()
and .remove() are relevant, .detect() is not.
I think you simply want to use i2c_new_probed_device() instead of
i2c_new_device(), so that the i2c client is only instantiated if the
right device is present at the address. In the case where different
chips could be present and you have to differentiate between then, you
can provide a custom detection callback to i2c_new_probed_device(), and
have it return -ENODEV if the right device isn't found. If you don't
provide a custom detection callback, the default one is used
(i2c_probe_func_quick_read in i2c-core.c), that only checks for any
device's presence at a specified address.
Hope that clarifies,
Jean
On Tue, 15 Oct 2013 18:13:39 +0300, Antti Palosaari wrote:
> Jean
> could you look that and comment how I should implement it properly.
> I saw you also added i2c_new_probed_device() [1] that these TV drivers
> could be ported properly I2C model.
>
> [1] http://lists.lm-sensors.org/pipermail/i2c/2007-March/000990.html
>
> I am pretty sure I have looked and tested all the I2C pieces quite
> carefully. There is .detect() callback which looks just what I need, but
> unfortunately it is nor called in case of i2c_new_probed_device() and
> i2c_new_device().
>
>
>
> On 14.10.2013 05:31, Antti Palosaari wrote:
> > On 14.10.2013 03:10, Antti Palosaari wrote:
> >> kernel: usb 1-2: rtl2832u_tuner_attach:
> >> kernel: e4000 5-0064: e4000_probe:
> >> kernel: usb 1-2: rtl2832u_tuner_attach: client ptr ffff88030a849000
> >>
> >> See attached patch.
> >>
> >> Is there any way to return error to caller?
> >>
> >> Abuse platform data ptr from struct i2c_board_info and call
> >> i2c_unregister_device() ?
> >
> > Answer to myself: best option seems to be check i2c_get_clientdata()
> > pointer after i2c_new_device().
> >
> > client = i2c_new_device(&d->i2c_adap, &info);
> > if (client)
> > if (i2c_get_clientdata(client) == NULL)
> > // OOPS, I2C probe fails
> >
> > That is because it is set NULL in error case by really_probe() in
> > drivers/base/dd.c. Error status is also cleared there with comment:
> > /*
> > * Ignore errors returned by ->probe so that the next driver can try
> > * its luck.
> > */
> >
> > That is told in I2C documentation too:
> > Note that starting with kernel 2.6.34, you don't have to set the `data'
> > field
> > to NULL in remove() or if probe() failed anymore. The i2c-core does this
> > automatically on these occasions. Those are also the only times the core
> > will
> > touch this field.
> >
> >
> >
> > But maybe the comment for actual function, i2c_new_device, is still a
> > bit misleading as it says NULL is returned for the error. All the other
> > errors yes, but not for the I2C .probe() as it is reseted by device core.
> >
> > * This returns the new i2c client, which may be saved for later use with
> > * i2c_unregister_device(); or NULL to indicate an error.
> > */
> > struct i2c_client *
> > i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
> >
> >
> > regards
> > Antti
> >
> >
> >>
> >> regards
> >> Antti
> >>
> >> ---
> >> drivers/media/tuners/e4000.c | 31
> >> +++++++++++++++++++++++++++++++
> >> drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 18 ++++++++++++++++--
> >> 2 files changed, 47 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
> >> index 54e2d8a..f4e0567 100644
> >> --- a/drivers/media/tuners/e4000.c
> >> +++ b/drivers/media/tuners/e4000.c
> >> @@ -442,6 +442,37 @@ err:
> >> }
> >> EXPORT_SYMBOL(e4000_attach);
> >>
> >> +static int e4000_probe(struct i2c_client *client, const struct
> >> i2c_device_id *did)
> >> +{
> >> + dev_info(&client->dev, "%s:\n", __func__);
> >> + return -ENODEV;
> >> +}
> >> +
> >> +static int e4000_remove(struct i2c_client *client)
> >> +{
> >> + dev_info(&client->dev, "%s:\n", __func__);
> >> + return 0;
> >> +}
> >> +
> >> +static const struct i2c_device_id e4000_id[] = {
> >> + {"e4000", 0},
> >> + {}
> >> +};
> >> +
> >> +MODULE_DEVICE_TABLE(i2c, e4000_id);
> >> +
> >> +static struct i2c_driver e4000_driver = {
> >> + .driver = {
> >> + .owner = THIS_MODULE,
> >> + .name = "e4000",
> >> + },
> >> + .probe = e4000_probe,
> >> + .remove = e4000_remove,
> >> + .id_table = e4000_id,
> >> +};
> >> +
> >> +module_i2c_driver(e4000_driver);
> >> +
> >> MODULE_DESCRIPTION("Elonics E4000 silicon tuner driver");
> >> MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
> >> MODULE_LICENSE("GPL");
> >> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> >> b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> >> index defc491..fbbe867 100644
> >> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> >> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> >> @@ -898,8 +898,22 @@ static int rtl2832u_tuner_attach(struct
> >> dvb_usb_adapter *adap)
> >> adap->fe[0]->ops.tuner_ops.get_rf_strength;
> >> return 0;
> >> case TUNER_RTL2832_E4000:
> >> - fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
> >> - &rtl2832u_e4000_config);
> >> +// fe = dvb_attach(e4000_attach, adap->fe[0], &d->i2c_adap,
> >> +// &rtl2832u_e4000_config);
> >> + {
> >> + static const struct i2c_board_info info = {
> >> + .type = "e4000",
> >> + .addr = 0x64,
> >> + };
> >> + struct i2c_client *client;
> >> +
> >> + fe = NULL;
> >> + client = i2c_new_device(&d->i2c_adap, &info);
> >> + if (IS_ERR_OR_NULL(client))
> >> + dev_err(&d->udev->dev, "e4000 probe failed\n");
> >> +
> >> + dev_dbg(&d->udev->dev, "%s: client ptr %p\n", __func__,
> >> client);
> >> + }
> >> break;
> >> case TUNER_RTL2832_FC2580:
> >> fe = dvb_attach(fc2580_attach, adap->fe[0], &d->i2c_adap,
> >>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-15 17:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-14 0:10 cannot ret error from probe - switch tuner to I2C driver model Antti Palosaari
2013-10-14 2:31 ` Antti Palosaari
2013-10-15 15:13 ` Antti Palosaari
2013-10-15 17:11 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox