From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keerthy Subject: Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF Date: Thu, 8 Jun 2017 10:01:19 +0530 Message-ID: References: <1496760340-27959-1-git-send-email-j-keerthy@ti.com> <1496760340-27959-4-git-send-email-j-keerthy@ti.com> <20170607103753.n3entu2ylxvhdax7@dell> <20170607103839.2bydxqxussxyahvn@dell> <41600d7d-3754-2907-151e-57fa20f88222@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Javier Martinez Canillas Cc: Enric Balletbo Serra , Lee Jones , Tony Lindgren , Milo Kim , "linux-omap@vger.kernel.org" , linux-kernel List-Id: linux-omap@vger.kernel.org On Wednesday 07 June 2017 07:40 PM, Javier Martinez Canillas wrote: > Hello Keerthy, > > On Wed, Jun 7, 2017 at 3:45 PM, Keerthy wrote: > > [snip] > >>>>>>>>> >>>>>>>> >>>>>>>> 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. >>>> > > That's correct... That is what i wanted to know. Thanks. > >>> >>> 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; >> > > ...but this particular driver only has a single entry in the OF table > and so you can just do: > > tps->id = TPS65217; > > Later if there's a variant for this chip, then you can add the logic > to query the struct of_device_id .data. But for now I think that's > better to just remove as Enric proposes and also remove the .data > field from the struct of_device_id entry. okay agreed for tps65217. > > Best regards, > Javier >