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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D64F0C6FD1F for ; Tue, 14 Mar 2023 18:29:07 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id D496185BFE; Tue, 14 Mar 2023 19:29:02 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=metanate.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=metanate.com header.i=@metanate.com header.b="gySwT1mj"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id BCBF185E8F; Tue, 14 Mar 2023 19:28:59 +0100 (CET) Received: from metanate.com (unknown [IPv6:2001:8b0:1628:5005::111]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id E9A8A858DA for ; Tue, 14 Mar 2023 19:28:55 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=metanate.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=john@metanate.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=metanate.com; s=stronger; h=In-Reply-To:Content-Type:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description; bh=aJXc5h7Jf1BVCZzYkSEu7xo8PufuV1E1GLrrFyOzMCo=; b=gySwT 1mjg7wwuvQVi09umgZwDShwW9NU6dQlDhEbpEaRknXMuipd1v1t0qEnD2vEbaN/zGGYllyntrB7zk xVUmYrepyV4wREqrMoye1CQp5ECFvME6584fouYDgBj7sxzFrLG+NvJLY2YGfCEMdQypK336bCstk jS/cmRIVi8rKc/i5q7JlLxzC+yCgDQIU+ZAlLZjB58xMpqOFribJBI47I67RbPs0J7tHAB2cpBrZ7 fQoVxc3lHKswm75+iTBlby9G9U6wfxftw0JYETweAncgC4T0rPWmUagWjjacJwi6ZgP+n8vt+s9BW ZJYMT/nbfFilOHWs6yFCCkrSdOHgw==; Received: from [81.174.171.191] (helo=donbot) by email.metanate.com with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pc9OB-0002MP-Rd; Tue, 14 Mar 2023 18:28:47 +0000 Date: Tue, 14 Mar 2023 18:28:45 +0000 From: John Keeping To: Johan Jonker Cc: dario.binacchi@amarulasolutions.com, michael@amarulasolutions.com, sjg@chromium.org, philipp.tomsich@vrull.eu, kever.yang@rock-chips.com, u-boot@lists.denx.de, yifeng.zhao@rock-chips.com Subject: Re: [PATCH v8 13/24] rockchip: rk3288: syscon_rk3288: store syscon platdata in regmap Message-ID: References: <6e68b95b-bf55-647d-df5e-cbbce20257f0@gmail.com> <8fcf5d3f-185b-7f94-25c9-e4f2ce0562a5@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8fcf5d3f-185b-7f94-25c9-e4f2ce0562a5@gmail.com> X-Authenticated: YES X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On Mon, Mar 13, 2023 at 10:09:23PM +0100, Johan Jonker wrote: > On 3/13/23 18:46, John Keeping wrote: > > On Mon, Mar 13, 2023 at 05:53:20PM +0100, Johan Jonker wrote: > >> On 3/13/23 14:26, John Keeping wrote: > >>> On Mon, Mar 13, 2023 at 01:30:57AM +0100, Johan Jonker wrote: > >>>> The Rockchip SoC rk3288 has 2 types of device trees floating around. > >>>> A 64bit reg size when synced from Linux and a 32bit for U-boot. > >>>> A pre-probe function in the syscon class driver assumes only 32bit. > >>>> For other odd reg structures the regmap must be defined in the individual > >>>> syscon driver. Store rk3288 platdata in a regmap before pre-probe > >>>> during bind. > >>>> > >>>> Signed-off-by: Johan Jonker > >>>> --- > >>> > >>> What is special about the rk3288 syscon that means the driver needs this > >>> handling? Isn't this a general problem for DTs with 64-bit addresses on > >>> 32-bit systems that could be solved in syscon-uclass.c? > >> > >> The dtd structure is only know to the driver with the SoC orientated compatible string. > >> I see guessing the "reg" size more as a legacy that we keep using for existing drivers > >> and should be deprecated. > >> > >>> > >>> I suspect it's difficult to handle the general case since #memory-cells > >>> may be difference for difference syscons so a global constant doesn't > >>> work, but the approach in this patch seems incredibly verbose for > >> > >> You are right here, but other then rk3288 I don't see that happen for other 32bit Rockchip SoCs. > > >> It's more verbose, because struct syscon_uc_info is not there yet in the bind phase. (ie. calloc) > > Currently syscon_uc_info is allocated/set after bind on in the probe phase. > > device_probe()->device_of_to_plat()->device_alloc_priv()->dev_set_uclass_priv() > > Not aware how to hookup to "struct uclass_driver",so no other option to do that then here. > > > > > What about non-Rockchip SoCs using the syscon uclass? > > > >>> something that is likely to be needed for many platforms. > >>> > >> > >>> Could we use driver flags with something like: > >>> > >>> .flags = of_platdata_reg_size(struct rockchip_rk3288_noc_plat), > >> > >> Driver flags might solve only the "reg" size part, but not the > >> ARRAY_SIZE and the unknown "reg" property location part. > > > > > Right - but the generic syscon-uclass code assumes a single range and > > that reg is the first property (at least I think it should be assuming a > > single range, but it looks like there might be a bug there now). > > I'm not aware that syscon nodes can have multiple reg ranges, but don't assume that in the future there won't be a binding that does. > > > > >>> and this untested macro: > >>> > >>> #define of_platdata_reg_size(s) \ > >>> ((sizeof(((struct rockchip_rk3288_noc_plat *) 0)->reg) == 64) ? \ > >>> DM_FLAGS_PLATDATA_REG_64BIT : 0) > >> > >> This would create a parallel data flow of a "size flag and ARRAY_SIZE variable + data" in > >> a structure to the syscon class driver that also must be stored somewhere, > >> while we could do the thing correct in the regmap structure right away. > > > > But the syscon uclass doesn't need any of that extra info - for > > OF_PLATDATA is makes assumptions and I don't see why that needs to be > > any different for 32-bit platforms with #memory-cells = <2>. From > > syscon-uclass.c: > > > > /* > > * With OF_PLATDATA we really have no way of knowing the format of > > * the device-specific platform data. So we assume that it starts with > > * a 'reg' member, and this holds a single address and size. Drivers > > * using OF_PLATDATA will need to ensure that this is true. > > */ > > > > In fact, for RK3288 we can just do: > > > > > static_assert(sizeof(fdt_val_t) == FIELD_SIZEOF(struct dtd_rockchip_rk3288_grf, reg[0])); > > fdt_val_t describes the parser capability and is not the same as the size in the dtd structure. > > > > > and the uclass gets this right. > > There's no guaranty that the dtd structure is going to be like syscon_base_plat and that the reg property is first. > > struct syscon_base_plat { > phys_addr_t reg[2]; > }; > > From syscon.yaml and other bindings we learn that there can be other properties then "reg" in syscon nodes. > Properties pop-up where ever they like in the dtd structure after combining various dtsi and dts files. > > struct dtd_rockchip_rk3066_grf { > bool dummy_property; > fdt64_t reg[2]; > }; > > Only passing a size flag is not enough. What I still don't understand is how any of that is different from the current state - so why does this need to change to support 64-bit addresses? All of the constraints you discuss above are mentioned in the existing uclass-syscon.c implementation, but that implementation is working today for all of these platforms with 32-bit addresses. So is there any need to support all those other use cases as part of allowing 64-bit addresses in device trees?