Re: [PATCH v1] i2c: icy: Don't use software node when it's an overkill

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Thu, Apr 09, 2020 at 01:37:35PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 08, 2020 at 11:24:20PM +0200, Max Staudt wrote:
> > Thank you, that looks much nicer indeed. I have also tested it on my Amiga just to be sure - it works.
> 
> Thank you.
> 
> > However, no signature yet because the call stack is giving me a headache:
> > 
> > icy_probe()
> >  -> i2c_new_scanned_device()
> >  -> i2c_new_client_device()
> >  -> device_add_properties()
> > 
> > And here we are in drivers/base/property.c looking at device_add_properties():
> > 
> >  * WARNING: The callers should not use this function if it is known that there
> >  * is no real firmware node associated with @dev! In that case the callers
> >  * should create a software node and assign it to @dev directly.
> > 
> > 
> > Why is this warning there? It flies right in the face of what we're trying to achieve here.
> > 
> > It was introduced in 2018 with commit caf35cd52242 .
> > 
> > 
> > So either the warning is superfluous, or i2c_new_client_device() should be creating a software fwnode, I guess?
> 
> No and no.
> 
> First one because the mechanism is added to have quirks, it must not replace
> the actual possibility to provide this via firmware (DT / ACPI).
> 
> Second one, because software node API should have (has?) the same warning.
> 
> +Cc Heikki.
> 
> Heikki, am I correct?

In this case it should be possible supply a handle to a software node
with the board info. That should then later replace the fwnode and
properties members once the existing code is converted:

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index f6b942150631..648ea384d480 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -409,8 +409,7 @@ struct i2c_board_info {
        const char      *dev_name;
        void            *platform_data;
        struct device_node *of_node;
-       struct fwnode_handle *fwnode;
-       const struct property_entry *properties;
+       const struct software_node *swnode
        const struct resource *resources;
        unsigned int    num_resources;
        int             irq;

I2C core would then need to take care of registering that swnode of
course.

thanks,

-- 
heikki