Dealing with optional i2c devices in a devicetree

Dealing with optional i2c devices in a devicetree

[Chris Packham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1490 bytes --]

Hi,

I'm working on a new board and one feature it as is a plug-in module 
with an ADS7830 voltage monitor on it. This will be used during 
manufacturing to sanity check that various voltage rails are within 
expected ranges.

I have a dts entry for the device as below (with some omissions for the 
sake of clarity)

	soc {
		internal-regs {
			i2c@11000 {
				ads7830@48 {
					compatible = "ads7830";
					reg = <0x48>;
				};
			};
		};
	};

The problem is that when the manufacturing card is not installed the 
device still shows up in /sys/class/hwmon/hwmon0/ and 
/sys/bus/i2c/devices/0-0048/ despite it not actually being present. If I 
was using an old style initialization I could use 
i2c_new_probed_device() which I think would stop the drivers probe() 
function from being called

Looking at the ads7828_probe() function it doesn't actually do anything 
with the i2c device before calling hwmon_device_register(). Some hwmon 
drivers like lm73_probe() do attempt to read from the device and bail if 
the read fails. I can probably fix my problem by doing something similar 
in the ads7828_probe(), but there are other drivers that have a similar 
probe function.

Is there a better way of getting the devicetree machinery to avoid the 
call to the driver probe function in the first place?

Thanks,
ChrisN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

Re: Dealing with optional i2c devices in a devicetree

[Arnd Bergmann]

On Friday 05 June 2015 06:03:30 Chris Packham wrote:
> 
> Is there a better way of getting the devicetree machinery to avoid the 
> call to the driver probe function in the first place?
> 
> 

The newly added DT overlay support should do what you need, but it might
not be the easiest solution.

To start out with the device not getting probed at first, add a
'status="disabled"' property, which will prevent the i2c_device from
getting added. At runtime you then need a way to change that from
"disabled" to "ok" and retrigger the probe. I haven't done that myself,
so it might need some i2c core changes to work correctly.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Dealing with optional i2c devices in a devicetree

[Guenter Roeck]

Hi Chris,

On 06/04/2015 11:03 PM, Chris Packham wrote:
> Hi,
>
> I'm working on a new board and one feature it as is a plug-in module
> with an ADS7830 voltage monitor on it. This will be used during
> manufacturing to sanity check that various voltage rails are within
> expected ranges.
>
> I have a dts entry for the device as below (with some omissions for the
> sake of clarity)
>
> 	soc {
> 		internal-regs {
> 			i2c@11000 {
> 				ads7830@48 {
> 					compatible = "ads7830";
> 					reg = <0x48>;
> 				};
> 			};
> 		};
> 	};
>
> The problem is that when the manufacturing card is not installed the
> device still shows up in /sys/class/hwmon/hwmon0/ and
> /sys/bus/i2c/devices/0-0048/ despite it not actually being present. If I
> was using an old style initialization I could use
> i2c_new_probed_device() which I think would stop the drivers probe()
> function from being called
>
> Looking at the ads7828_probe() function it doesn't actually do anything
> with the i2c device before calling hwmon_device_register(). Some hwmon
> drivers like lm73_probe() do attempt to read from the device and bail if
> the read fails. I can probably fix my problem by doing something similar
> in the ads7828_probe(), but there are other drivers that have a similar
> probe function.
>
But that would be artificial in this case, and it could not be used to really
detect the chip but would only prove that there is a chip at the selected i2c
address. This is not really useful.

> Is there a better way of getting the devicetree machinery to avoid the
> call to the driver probe function in the first place?
>

The easiest solution would probably be to drop the entry from the
devicetree file and instantiate the driver manually via sysfs when
needed. Another option would be to load a different devicetree file
if the chip is plugged in.

You might also be able to use a devicetree overlay, but that would
probably add too much complexity if the chip is only used in
manufacturing.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Dealing with optional i2c devices in a devicetree

[Jean Delvare]

Hi Chris,

On Fri, 5 Jun 2015 06:03:30 +0000, Chris Packham wrote:
> I'm working on a new board and one feature it as is a plug-in module 
> with an ADS7830 voltage monitor on it. This will be used during 
> manufacturing to sanity check that various voltage rails are within 
> expected ranges.
> 
> I have a dts entry for the device as below (with some omissions for the 
> sake of clarity)
> 
> 	soc {
> 		internal-regs {
> 			i2c@11000 {
> 				ads7830@48 {
> 					compatible = "ads7830";
> 					reg = <0x48>;
> 				};
> 			};
> 		};
> 	};
> 
> The problem is that when the manufacturing card is not installed the 
> device still shows up in /sys/class/hwmon/hwmon0/ and 
> /sys/bus/i2c/devices/0-0048/ despite it not actually being present. If I 
> was using an old style initialization I could use 
> i2c_new_probed_device() which I think would stop the drivers probe() 
> function from being called

It would prevent the i2c device from being instantiated in the first
place, which in turn indeed means no probe function would be called.

> Looking at the ads7828_probe() function it doesn't actually do anything 
> with the i2c device before calling hwmon_device_register(). Some hwmon 
> drivers like lm73_probe() do attempt to read from the device and bail if 
> the read fails. I can probably fix my problem by doing something similar 
> in the ads7828_probe(), but there are other drivers that have a similar 
> probe function.

The lm73 driver needs to read a register value in order to initialize
the internal device state. If the read fails, something like -EIO will
be returned, not -ENODEV. In other words, the lm73 driver reports that
it failed to initialize the device, NOT that there was no device. And
this will not remove the "lm73" i2c device. Just no hwmon class device
will be created.

> Is there a better way of getting the devicetree machinery to avoid the 
> call to the driver probe function in the first place?

There is nothing hwmon-specific in your question. You need a way to
declare in the device tree devices which may or may not be present. Or
you need a way to modify the device tree at run-time depending on which
hardware plug-in modules are present. I have no idea if either can be
done, this is really a question for people familiar with the device
tree model and infrastructure (which I am not, sorry.)

-- 
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Dealing with optional i2c devices in a devicetree

[Chris Packham]

On 06/05/2015 07:26 PM, Arnd Bergmann wrote:
> On Friday 05 June 2015 06:03:30 Chris Packham wrote:
>>
>> Is there a better way of getting the devicetree machinery to avoid the
>> call to the driver probe function in the first place?
>>
>>
>
> The newly added DT overlay support should do what you need, but it might
> not be the easiest solution.
>
> To start out with the device not getting probed at first, add a
> 'status="disabled"' property, which will prevent the i2c_device from
> getting added. At runtime you then need a way to change that from
> "disabled" to "ok" and retrigger the probe. I haven't done that myself,
> so it might need some i2c core changes to work correctly.
>

So I pursued this idea a little.

I set the node to status = "disabled" in the dts. Then I added some code 
to probe for the i2c device using
i2c_probe_func_quick_read() and updated the device tree to remove the 
status property based on the result of the probe.

Then I was able to call of_platform_populate(node->parent, ...). This 
got me as far as the platform device being created but none of the i2c 
or hwmon code kicked in. I may have missed something, perhaps the 
platform bus needs to be re-probed now that there is a new device.

I think I'll look at updating the device tree passed by the bootloader 
which should be easier to do at this point.

For future (and current) mailing list readers this is the gist of my 
detect function.

static void board_detect(struct device *dev)
{
	struct i2c_adapter *adap = i2c_get_adapter(0);

	/* Ugly hack. The ads7830 device is a SMBUS device at 0x48.
	 * SMBUS is not i2c but the quick read should be OK-ish.
	 */
	if (i2c_probe_func_quick_read(adap, 0x48)) {
		int ret;
		struct device_node *node;

		dev_info(dev, "board detected");
		for_each_compatible_node(node, NULL, "ads7830") {
			struct property *prop;
			const struct of_device_id match_table[] = {
				{ .compatible = "ads7830", },
				{} /* Empty terminated list */
			};

			prop = of_find_property(node, "status", NULL);
			if (prop)
				of_remove_property(node, prop);

			ret = of_platform_populate(node->parent,
						   match_table,
						   NULL, NULL);
			if (ret)
				dev_err(dev, "populate failed\n");
		}
	}

	i2c_put_adapter(adap);
	return;
}--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Dealing with optional i2c devices in a devicetree

[Chris Packham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2534 bytes --]

Thanks to those who replied.

On 06/05/2015 07:30 PM, Guenter Roeck wrote:
> Hi Chris,
>
> On 06/04/2015 11:03 PM, Chris Packham wrote:
>> Hi,
>>
>> I'm working on a new board and one feature it as is a plug-in module
>> with an ADS7830 voltage monitor on it. This will be used during
>> manufacturing to sanity check that various voltage rails are within
>> expected ranges.
>>
>> I have a dts entry for the device as below (with some omissions for the
>> sake of clarity)
>>
>>     soc {
>>         internal-regs {
>>             i2c@11000 {
>>                 ads7830@48 {
>>                     compatible = "ads7830";
>>                     reg = <0x48>;
>>                 };
>>             };
>>         };
>>     };
>>
>> The problem is that when the manufacturing card is not installed the
>> device still shows up in /sys/class/hwmon/hwmon0/ and
>> /sys/bus/i2c/devices/0-0048/ despite it not actually being present. If I
>> was using an old style initialization I could use
>> i2c_new_probed_device() which I think would stop the drivers probe()
>> function from being called
>>
>> Looking at the ads7828_probe() function it doesn't actually do anything
>> with the i2c device before calling hwmon_device_register(). Some hwmon
>> drivers like lm73_probe() do attempt to read from the device and bail if
>> the read fails. I can probably fix my problem by doing something similar
>> in the ads7828_probe(), but there are other drivers that have a similar
>> probe function.
>>
> But that would be artificial in this case, and it could not be used to
> really
> detect the chip but would only prove that there is a chip at the
> selected i2c
> address. This is not really useful.
>
>> Is there a better way of getting the devicetree machinery to avoid the
>> call to the driver probe function in the first place?
>>
>
> The easiest solution would probably be to drop the entry from the
> devicetree file and instantiate the driver manually via sysfs when
> needed. Another option would be to load a different devicetree file
> if the chip is plugged in.
>
> You might also be able to use a devicetree overlay, but that would
> probably add too much complexity if the chip is only used in
> manufacturing.

Sounds like my best bet is to mark the nodes as disabled in the dts and 
have my bootloader update them on the way through.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

Re: Dealing with optional i2c devices in a devicetree

[Enrico Weigelt, metux IT consult]

Am 08.06.2015 um 08:01 schrieb Chris Packham:

Hi folks,

 > Sounds like my best bet is to mark the nodes as disabled in the
 > dts and have my bootloader update them on the way through.

In case your bootloader can take that decision (eg. if the device
is present at boot time, and the bootloader has the proper probing
logic or simply knows the device has to be there), that would be
an pretty easy way.


But let me add another usecase, which might be a bit more tricky:

Let's assume we can plug in some more complex device, which consists
of several (pretty standard) subdevices, behind certain bus'es (maybe
it's attached via USB, and somewhere behind are some I2C bus'es with
regulators, pwm-generators, etc).

Let's further assume, we already got some DTS or some piece of memory
with an DTB subtree for that device (eg. some simple bulk endpoint that
just gives back a bunch of bytes with the DTB).

Now, when the device is plugged in, I'd like to get that piece of DTB
loaded and the corresponding drivers initialized. And, of course, when
it's plugged-out, everything should be shut down cleanly.

How could we achieve that ?


--mtx
--
Enrico Weigelt, metux IT consult
+49-151-27565287
MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B

Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur für einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschließlich für denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, dürfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrtümlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte löschen Sie in diesem Fall diese Nachricht und alle Anhänge, ohne eine Kopie zu behalten.
Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Dealing with optional i2c devices in a devicetree

[Guenter Roeck]

On 06/08/2015 05:11 AM, Enrico Weigelt, metux IT consult wrote:
> Am 08.06.2015 um 08:01 schrieb Chris Packham:
>
> Hi folks,
>
>  > Sounds like my best bet is to mark the nodes as disabled in the
>  > dts and have my bootloader update them on the way through.
>
> In case your bootloader can take that decision (eg. if the device
> is present at boot time, and the bootloader has the proper probing
> logic or simply knows the device has to be there), that would be
> an pretty easy way.
>
>
> But let me add another usecase, which might be a bit more tricky:
>
> Let's assume we can plug in some more complex device, which consists
> of several (pretty standard) subdevices, behind certain bus'es (maybe
> it's attached via USB, and somewhere behind are some I2C bus'es with
> regulators, pwm-generators, etc).
>
> Let's further assume, we already got some DTS or some piece of memory
> with an DTB subtree for that device (eg. some simple bulk endpoint that
> just gives back a bunch of bytes with the DTB).
>
> Now, when the device is plugged in, I'd like to get that piece of DTB
> loaded and the corresponding drivers initialized. And, of course, when
> it's plugged-out, everything should be shut down cleanly.
>
> How could we achieve that ?
>

Use devicetree overlays.

Guenter


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html