From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752604AbcGCLh7 (ORCPT ); Sun, 3 Jul 2016 07:37:59 -0400 Received: from mail2.asahi-net.or.jp ([202.224.39.198]:12211 "EHLO mail2.asahi-net.or.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751867AbcGCLh5 (ORCPT ); Sun, 3 Jul 2016 07:37:57 -0400 Date: Sun, 03 Jul 2016 20:37:56 +0900 Message-ID: <87shvrrlyj.wl-ysato@users.sourceforge.jp> From: Yoshinori Sato To: Lee Jones Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH RESEND v2] mfd: sm501: Add device property In-Reply-To: <20160701085911.GJ1707@dell> References: <1467219419-22901-1-git-send-email-ysato@users.sourceforge.jp> <20160630074800.GE1707@dell> <87wpl6d60k.wl-ysato@users.sourceforge.jp> <20160701085911.GJ1707@dell> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL/10.8 EasyPG/1.0.0 Emacs/24.5 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-2022-JP Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 01 Jul 2016 17:59:11 +0900, Lee Jones wrote: > > On Fri, 01 Jul 2016, Yoshinori Sato wrote: > > > On Thu, 30 Jun 2016 16:48:00 +0900, > > Lee Jones wrote: > > > > > > On Thu, 30 Jun 2016, Yoshinori Sato wrote: > > > > > > > Signed-off-by: Yoshinori Sato > > > > --- > > > > Documentation/devicetree/bindings/mfd/sm501.txt | 45 +++++++++++++++++++++++++ > > > > drivers/mfd/sm501.c | 9 +++++ > > > > 2 files changed, 54 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/mfd/sm501.txt > > [...] > > > > > diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c > > > > index 65cd0d2..e2e3f9b 100644 > > > > --- a/drivers/mfd/sm501.c > > > > +++ b/drivers/mfd/sm501.c > > [...] > > > > > @@ -1377,6 +1378,8 @@ static int sm501_plat_probe(struct platform_device *dev) > > > > { > > > > struct sm501_devdata *sm; > > > > int ret; > > > > + struct sm501_platdata private_platdata; > > > > + struct sm501_initdata private_initdata; > > > > > > > > sm = kzalloc(sizeof(struct sm501_devdata), GFP_KERNEL); > > > > if (sm == NULL) { > > > > @@ -1388,6 +1391,12 @@ static int sm501_plat_probe(struct platform_device *dev) > > > > sm->dev = &dev->dev; > > > > sm->pdev_id = dev->id; > > > > sm->platdata = dev_get_platdata(&dev->dev); > > > > + if (!sm->platdata) { > > > > + of_property_read_u32(dev->dev.of_node, "smi,devices", > > > > + (u32 *)&private_initdata.devices); > > > > + private_platdata.init = &private_initdata; > > > > + sm->platdata = &private_platdata; > > > > + } > > > > > > I've asked about this 3 times now. > > > > > > What consumes this platform data? > > > > > > It also looks ugly and fragile. > > > > It's appropriate to use dev.of_node, isn't it? > > If it's misunderstood, I'm sorry. > > Yes, that's acceptable. > > I'm talking about the whole process of allocating an entire platform > structure (on the stack, which will most likely be wiped when we > return from probe()) just to populate this one, undocumented > property. > > Hold up ... I've just taken a look at the driver myself. What a > mess. It appears this driver pre-dates the MFD API and does > everything I hate. > > First step is to see if the Device Tree guys like your new property. > Please document it in bindings/display/sm501fb.txt, as suggested by > Rob, so they can review it. Don't forget to CC me too. > > We can look at the C code changes later. OK. Thanks comment. > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog -- Yoshinori Sato