From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 615CBC7EE25 for ; Mon, 12 Jun 2023 13:19:21 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 99EC610E237; Mon, 12 Jun 2023 13:19:20 +0000 (UTC) Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id 89E2610E237 for ; Mon, 12 Jun 2023 13:19:18 +0000 (UTC) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C779CCE6; Mon, 12 Jun 2023 15:18:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1686575928; bh=Bj+SpMG81sTj8aensnQvokjjL3Rwu9dHwfJTAZQIrBk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=unso4Umz7SnrFwQD6vncMYiN8Wghu0zrDkXYhTxYVqPsvHrha9zPuyquvf4leAAsY aP6tl5rSEy52aLPeBmoU19RN6n2unwxLxcfM7a45sx1ngyujr8w2ddKgM30gZBzw7E jipNHUxMxAAHkEqHwP9VbTz9Xv/hkHgz7DA7P+gA= Date: Mon, 12 Jun 2023 16:19:17 +0300 From: Laurent Pinchart To: Geert Uytterhoeven Subject: Re: [PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device API Message-ID: <20230612131917.GD23921@pendragon.ideasonboard.com> References: <20230608103929.GO5058@pendragon.ideasonboard.com> <20230608125019.GD26742@pendragon.ideasonboard.com> <20230612122353.GA22391@pendragon.ideasonboard.com> <20230612125405.GA23921@pendragon.ideasonboard.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Corey Minyard , Alexandre Belloni , Andrzej Hajda , Antonio Borneo , "dri-devel@lists.freedesktop.org" , "linux-i2c@vger.kernel.org" , Krzysztof Kozlowski , Trent Piepho , Marek =?utf-8?B?QmVow7pu?= , "linux-renesas-soc@vger.kernel.org" , Robert Foss , Jonas Karlman , Kieran Bingham , Jernej Skrabec , Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= , "linux-media@vger.kernel.org" , Conor Dooley , Alessandro Zummo , Jiasheng Jiang , Abhinav Kumar , Fabrizio Castro , Rob Herring , Biju Das , Ahmad Fatoum , Mauro Carvalho Chehab , Neil Armstrong , Wolfram Sang , Mark Brown , Hans Verkuil Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Geert, On Mon, Jun 12, 2023 at 03:08:46PM +0200, Geert Uytterhoeven wrote: > On Mon, Jun 12, 2023 at 2:54 PM Laurent Pinchart wrote: > > On Mon, Jun 12, 2023 at 12:42:33PM +0000, Biju Das wrote: > > > > Subject: Re: [PATCH v5 01/11] i2c: Enhance i2c_new_ancillary_device API > > > > On Mon, Jun 12, 2023 at 09:53:02AM +0000, Biju Das wrote: > > > > > How do we proceed here between [1] and [2]? > > > > > > > > > > DT-Maintainers suggestion: > > > > > [1] > > > > > raa215300: pmic@12 { > > > > > compatible = "renesas,raa215300"; > > > > > reg = <0x12>, <0x6f>; > > > > > reg-names = "main", "rtc"; > > > > > > > > > > clocks = <&x2>; > > > > > clock-names = "xin"; > > > > > /* Add Optional shared IRQ resource and share it to child and handle > > > > > it both in parent and child */ }; > > > > > > > > > > Laurent/Wolfram suggestion to split it into two nodes and get rid of this patch: > > > > > [2] > > > > > raa215300: pmic @12 { > > > > > compatible = "renesas,raa215300"; > > > > > reg = <0x12>; > > > > > > > > > > /* Add Optional shared IRQ */ > > > > > renesas,raa215300-rtc = <&rtc_raa215300>; /* Parse the handle and Enable RTC , if present.*/ > > > > > }; > > > > > > > > > > rtc_raa215300: rtc@6f { > > > > > compatible = "renesas,raa215300-isl1208"; > > > > > > > > Make this > > > > > > > > compatible = "renesas,raa215300-isl1208", "isil,isl1208"; > > > > > > > > > reg = <0x6f>; > > > > > > > > > > /* Add Optional shared IRQ */ > > > > > clocks = <&x2>; > > > > > clock-names = "xin"; > > > > > renesas,raa215300-pmic = <&pmic>; /* Parse the handle to get PMIC > > > > > version to check Oscillator bit is inverted or not */ > > > > > > > > This isn't nice. I would instead add a renesas,invert-xtoscb boolean > > > > property. If you don't want different DT sources for different revisions > > > > of the PMIC, > > > > > > I need to support all PMIC versions with same image, as PMIC is just a component on the > > > SoM module. So SoM's have different PMIC versions. > > > > I understand it's not convenient, so let's try to find a good solution. > > > > > > one option is to perform the auto-detection in the boot > > > > loader and update the DT dynamically there. > > > > > > Yes, this is an option. Bootloader updates "renesas,invert-xtoscb" property based > > > on PMIC version. > > > > > > Not sure, From binding perspective, Documenting "renesas,invert-xtoscb" is OK for > > > the relevant maintainers?? > > > > It's fine with me at least :-) I think a property makes sense, as it > > describes the device. Updating the device tree in the boot loader based > > on auto-detection of features is also fairly common (to set the amount > > of DRAM for instance). > > > > What I'm not entirely sure about in this case is if a property would be > > the best option, or two different compatible strings. I'll let the > > appropriate maintainer recommend one of those two options. In either > > case, the boot loader would be responsible for updating the DT. > > Indeed. DT binding best practices 101: do not use properties to > distinguish, use compatible values instead. > > And don't use different compatible values if you can distinguish using > a version register. Unfortunately the version register is part of the > main/first device (the PMIC), so the RTC cannot find out easily... That's not very different from having IP cores whose integration is different between different SoC versions. We could easily add SoC match code in drivers and map the SoC version to integration data, but it's not a good practice. DT helps decoupling integration (and quirks) from drivers and allows getting rid of lots of cross-driver communication (which used to be handled through board files). > So basically you have an i2c mfd. The Linux mfd subsystem is tailored > for platform devices, so it's not a good match. The closest we have > in i2c is the ancillary device... I think an MFD-type solution that's way too much trouble to handle the issue at hand. I recommend a DT property here. -- Regards, Laurent Pinchart