On Wednesday 07 June 2017 07:07 PM, Enric Balletbo Serra wrote: > 2017-06-07 13:24 GMT+02:00 Keerthy <j-keerthy@xxxxxx>: >> >> >> On Wednesday 07 June 2017 04:08 PM, Lee Jones wrote: >>> On Wed, 07 Jun 2017, Lee Jones wrote: >>> >>>> On Wed, 07 Jun 2017, Keerthy wrote: >>>> >>>>> >>>>> >>>>> On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote: >>>>>> Hi Keerthy, >>>>>> >>>>>> By change I was looking at this. Some comments below that I think can >>>>>> be applied to all patches in this series >>>>>> >>>>>> 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@xxxxxx>: >>>>>>> Currently the driver boots only via device tree hence add a >>>>>>> dependency on OF. >>>>>>> >>>>>>> Signed-off-by: Keerthy <j-keerthy@xxxxxx> >>>>>>> --- >>>>>>> drivers/mfd/Kconfig | 2 +- >>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>>>>>> index 75b59f1..2d1425d 100644 >>>>>>> --- a/drivers/mfd/Kconfig >>>>>>> +++ b/drivers/mfd/Kconfig >>>>>>> @@ -1297,7 +1297,7 @@ config MFD_TPS65090 >>>>>>> >>>>>>> config MFD_TPS65217 >>>>>>> tristate "TI TPS65217 Power Management / White LED chips" >>>>>>> - depends on I2C >>>>>>> + depends on I2C && OF >>>>>> >>>>>> Shouldn't you add || COMPILE_TEST here ? >>>>> >>>>> Sure. >>>>> >>>>>> >>>>>>> select MFD_CORE >>>>>>> select REGMAP_I2C >>>>>>> select IRQ_DOMAIN >>>>>>> >>>>>> >>>>>> I think you can remove the of_match_device checks in some drivers too >>>>>> >>>>>> i.e: >>>>>> >>>>>> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330 >>>>> >>>>> Yes that and removal of unused i2c_device_id. I will follow it up once >>>>> this OF dependency is in. >>>> >>>> The of_match_device() checks should be removed with the OF patch. >> >> Lee Jones/ Enric, >> >> IIUC of_match_device call is still needed to obtain a match and in case >> there are multiple compatibles with different match data then this call >> is definitely needed. >> > > Not sure if I follow you. My understanding is that with DT the probe > of this driver is only called if there is a node with the compatible = > "ti,tps65217" string. So if probe is called there is always a match > and the call to of_match_device is redundant. How will you get the matching data? For the tps65217 case you mentioned we need the match pointer to get the chip_id right? chip_id = (unsigned long)match->data; Also one more case of when we have multiple compatibles with different matching data. Ex: http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/palmas.c#L522 You need the match pointer to get the corresponding data. Hope i am clear. > >> There is no need to check for return value as we will find one match for >> sure and that can be removed. >> >> Even checks like 'if (client->dev.of_node) {' can surely be removed with >> depends on OF. >> > > Yes I think this should be removed too. > >> Please correct me if i am wrong. >> >> Regards, >> Keerthy >>> >>> In fact, just squash these changes into the I2C removal patches. >>> > > Regards, > Enric > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
