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 72C2BC6FD19 for ; Mon, 13 Mar 2023 21:09:32 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 8B29485BD1; Mon, 13 Mar 2023 22:09:29 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.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=gmail.com header.i=@gmail.com header.b="HCJxy6E5"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 5B5A685B75; Mon, 13 Mar 2023 22:09:28 +0100 (CET) Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id A8F7885B3D for ; Mon, 13 Mar 2023 22:09:25 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=jbx6244@gmail.com Received: by mail-ed1-x52e.google.com with SMTP id k10so54009966edk.13 for ; Mon, 13 Mar 2023 14:09:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678741765; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=y/7jViLqHzAl1fCS8dUeKN7x7h7YqEO+NUha9qUQDvY=; b=HCJxy6E55PsgJT8gNbAboo1E+aLVgMV2ipg6uMuOtXYNXkdAteKp2AiCjT8PmUy+84 PkJPZ7vGOyk4Y7Gvle8ZXlZThXg25t2Uwyz6+4S/IH2eiSfBxBVnl9XZhP+ZzDWx81rB R2V4emA9XeqHuVBQNZQZ7lHb0+LE5NRQihFsN9SV1k7nLNnDZv67aMlLqwCsv1qU+jbZ 3dUr9U3LjsHPICA731Qm3rMfd9IWIcrz/+8WWqHV6xn0sm5/zpQSMGuVXS05EfxkSyh7 r1QKJAsiXQiQImrVxDWHsak30xRjeN8aEzp0fWdaeWslOZrdw1USm3G0Qynwysd4x3qP C42g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678741765; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=y/7jViLqHzAl1fCS8dUeKN7x7h7YqEO+NUha9qUQDvY=; b=d3XoFIGigHUWEoLg4rf4PkM4zcr3mzQ++9qrUg3Borc2Era3lBhenGLgpX1iheOEEr vp5FLFh1NT131qa9Rpt9d3qIZWtcY2BqMULrwJXtgyFAtL95Y4MZX4Y7v+hsNz9XIoIk rcL2z+LQoQk6d2sCElj+QGIWc1n6ZAX5ApjKSOZr3aJQczxXZzBiH7FC+bFS80Jp7RoP Ugir5eo8WY0xm9aLWwa87kdyn92M6IAYNRi6EU+p9gHm/uACOmf2PtfvaBqFLEfWocMA hFlLDXK2J2draZAsNd6udEXlXW/SOm+JrsYTXfZ+hb53nf4ZO/U3LxN8VwPvuTpnnbux AFGQ== X-Gm-Message-State: AO0yUKXbhPXxDiYnSwzhZ4wzENvlxs2CH77M1b3PYnakyhy6xYGxwp+H /uT2e2DVnQOHvk59E3knjtI= X-Google-Smtp-Source: AK7set815jFZagi8wceqrAWOe/yk6e9red/xBYEjOSev3wxcNkvxkF0YgwbxSNddBdFfCz6SHoseKw== X-Received: by 2002:a17:907:6d87:b0:8f6:52c:afa0 with SMTP id sb7-20020a1709076d8700b008f6052cafa0mr42826641ejc.23.1678741764942; Mon, 13 Mar 2023 14:09:24 -0700 (PDT) Received: from [192.168.2.2] (81-204-249-205.fixed.kpn.net. [81.204.249.205]) by smtp.gmail.com with ESMTPSA id jz14-20020a17090775ee00b00926f89e2213sm225394ejc.190.2023.03.13.14.09.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 13 Mar 2023 14:09:24 -0700 (PDT) Message-ID: <8fcf5d3f-185b-7f94-25c9-e4f2ce0562a5@gmail.com> Date: Mon, 13 Mar 2023 22:09:23 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH v8 13/24] rockchip: rk3288: syscon_rk3288: store syscon platdata in regmap To: John Keeping 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 References: <6e68b95b-bf55-647d-df5e-cbbce20257f0@gmail.com> Content-Language: en-US From: Johan Jonker In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 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.