From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C690E170A0A for ; Fri, 29 Nov 2024 09:12:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732871574; cv=none; b=X90newEDks7OAUcFiQ7m7G5tvkp3txXrvQm7Ze58YO11xPiFtXDCePwJiGdFZN8GLILU625zWZeFbenx7PnA1D5VmJeEJnI56BB8/ZeIBSOtQQ7f7gIhaojJrrVEvUtpmCgo6A0v57v4RQ10OBw7vRXz2fHQug697/V9NrKLeho= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732871574; c=relaxed/simple; bh=1bnVnZ6g/RMKpFN6pROmGByLVFcvNA8/ZgUKEpHMa2Q=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=faZl/ZJ1oxHaD0306aU0GtHedgs6GM3BepPmvH0jId3cdbp400L6A138UDyTO6VRktPiz3SdIYP+xzglAHBVIkWb7GJ6mtCxMVi4No3w+5/9MWZnAbxrQzMHIWNtzHRS4n5adGBvqhyh3MjCDOIl1iGyjqEm8jWSpkOSu/Zd2Ak= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tuxon.dev; spf=pass smtp.mailfrom=tuxon.dev; dkim=pass (2048-bit key) header.d=tuxon.dev header.i=@tuxon.dev header.b=fFkN5daD; arc=none smtp.client-ip=209.85.128.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tuxon.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=tuxon.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tuxon.dev header.i=@tuxon.dev header.b="fFkN5daD" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-434a10588f3so10001095e9.1 for ; Fri, 29 Nov 2024 01:12:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tuxon.dev; s=google; t=1732871571; x=1733476371; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=sb8gO4uA+vo8kcJyPPhqX5i7haSv+q2JYHt3gAOv4Y0=; b=fFkN5daDx5C4IP+M2K9BheZExYOitRDImSD4IClty7KWVw6kGsNlBLBHmbFgRxZiuq lweIquyLSvl5YEt0Yn6J3MzsPMlYzothNseGTvwsxjS047fQ509z2gjZt71+/KA+c0hq UWkI5C98RxUSzkZTTEkLft223TIdESlYec/5KumR+fEKSo5zSnMsdpCvHihgmzLh+Eds em1M+nHzBCS1fWst4ueC7R4gX/BdxlWRHANeGgBKUj3HAxc6XdhiFi2BqxtRcOTbrQXv MgSjx3zFg1W5Hpby6+XL0KpqtZt7LMG+9djo6OukFi3NqaXL5hgcgD9ktYQQSQDKhVaD b0ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732871571; x=1733476371; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=sb8gO4uA+vo8kcJyPPhqX5i7haSv+q2JYHt3gAOv4Y0=; b=Y7eDvbAklczgvZSpv+WC1YksIl0G2cVJcBKHySAjvYCM0+bNSRHo2Zxp2ECWWCyS01 7sC3FqAWpgUUIZElgheMtBrj9oCtbVxJfllpm12KChYhoW5cJov5XNHvo0Lb/UlKX75T NqWMFPIrTH1jv/RsmmW8p+XNRX2DJq4RpvY2SASZUVpKJ60Y2I1uLjG9Qmh4n8C1hc11 qQl6W7XiEi12dY6foI72SUGUIeBYVBg8VS/1eLwU7VBqW/kS6pNSVU3TgbtJ8Zdu/uxF Eoj1ztDYyoPSi3QPCB5EsKhAPrY2EZPrHXm5t9SbHcxrhPRGj5hGYR1qlehukZCl9+Sq S7gA== X-Forwarded-Encrypted: i=1; AJvYcCX7IbykvNnIuNqvUI+1H6J4pI71vXvwXbxFlIqHPl4PXxnX7Ig8B+qCPwERwWdd1l6+vDgntiEQAWgLLBw=@vger.kernel.org X-Gm-Message-State: AOJu0YwwPx5dsDQQVkUbXqS9BiCfVD4VE3KictpMtXecS/vxkFCyh67c PHQzPGtAAhljpiP2cYPd8/Dx4//5oY2MV5ZC7YCJE/DOWINNbCYF+t3NDStTxuA= X-Gm-Gg: ASbGncvQrMiaTAoCdtATeqHcnBWOAIy1LmrI9+oWIUjO7hfGSrwqPjZNv0l/kl8qSIg AkvIp66fPcYFLWrOgX+Y+9iXZSXTeIu61Ijc7a2F2FxMMXHIvLJHKHLlDYIqHkkB91SpZOMNrcr VRTOz5OUsN7zKfq7Z8hzptemLyr2zh++m+JcbmK5BT6LfxA1bEkG7MKUOYoXAmlKqPk2h6vlCNN V8ARRy30DWaB5N7u75kK5g2j5EabHnzKBQWg4BBpKNo2v0en8rvfRiK1Q== X-Google-Smtp-Source: AGHT+IHmwDZAWFKDepsiDLw8yd17aIS/lp5WnEmXUs3bmN9CBmiHsuGbPOMSIZoIaRXxGX02f7LuEg== X-Received: by 2002:a05:600c:3b1a:b0:434:a968:89b5 with SMTP id 5b1f17b1804b1-434a9dc35f7mr102361905e9.9.1732871571093; Fri, 29 Nov 2024 01:12:51 -0800 (PST) Received: from [192.168.50.4] ([82.78.167.46]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-385ccd36a02sm3890362f8f.37.2024.11.29.01.12.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 29 Nov 2024 01:12:50 -0800 (PST) Message-ID: <4d2a4d8b-9951-4454-b662-0a14d73e61a0@tuxon.dev> Date: Fri, 29 Nov 2024 11:12:48 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 02/15] soc: renesas: Add SYSC driver for Renesas RZ family Content-Language: en-US To: Geert Uytterhoeven Cc: vkoul@kernel.org, kishon@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, p.zabel@pengutronix.de, magnus.damm@gmail.com, gregkh@linuxfoundation.org, yoshihiro.shimoda.uh@renesas.com, christophe.jaillet@wanadoo.fr, linux-phy@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-usb@vger.kernel.org, Claudiu Beznea References: <20241126092050.1825607-1-claudiu.beznea.uj@bp.renesas.com> <20241126092050.1825607-3-claudiu.beznea.uj@bp.renesas.com> <32fa7eb8-2139-454c-8866-cb264d060616@tuxon.dev> From: Claudiu Beznea In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 29.11.2024 10:54, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Fri, Nov 29, 2024 at 9:48 AM Claudiu Beznea wrote: >> On 28.11.2024 17:24, Geert Uytterhoeven wrote: >>> On Tue, Nov 26, 2024 at 10:21 AM Claudiu wrote: >>>> From: Claudiu Beznea >>>> >>>> The RZ/G3S system controller (SYSC) has various registers that control >>>> signals specific to individual IPs. IP drivers must control these signals >>>> at different configuration phases. >>>> >>>> Add SYSC driver that allows individual SYSC consumers to control these >>>> signals. The SYSC driver exports a syscon regmap enabling IP drivers to >>>> use a specific SYSC offset and mask from the device tree, which can then be >>>> accessed through regmap_update_bits(). >>>> >>>> Currently, the SYSC driver provides control to the USB PWRRDY signal, which >>>> is routed to the USB PHY. This signal needs to be managed before or after >>>> powering the USB PHY off or on. >>>> >>>> Other SYSC signals candidates (as exposed in the the hardware manual of the >>>> >>>> * PCIe: >>>> - ALLOW_ENTER_L1 signal controlled through the SYS_PCIE_CFG register >>>> - PCIE_RST_RSM_B signal controlled through the SYS_PCIE_RST_RSM_B >>>> register >>>> - MODE_RXTERMINATION signal controlled through SYS_PCIE_PHY register >>>> >>>> * SPI: >>>> - SEL_SPI_OCTA signal controlled through SYS_IPCONT_SEL_SPI_OCTA >>>> register >>>> >>>> * I2C/I3C: >>>> - af_bypass I2C signals controlled through SYS_I2Cx_CFG registers >>>> (x=0..3) >>>> - af_bypass I3C signal controlled through SYS_I3C_CFG register >>>> >>>> * Ethernet: >>>> - FEC_GIGA_ENABLE Ethernet signals controlled through SYS_GETHx_CFG >>>> registers (x=0..1) >>>> >>>> As different Renesas RZ SoC shares most of the SYSC functionalities >>>> available on the RZ/G3S SoC, the driver if formed of a SYSC core >>>> part and a SoC specific part allowing individual SYSC SoC to provide >>>> functionalities to the SYSC core. >>>> >>>> Signed-off-by: Claudiu Beznea >>> >>>> --- /dev/null >>>> +++ b/drivers/soc/renesas/r9a08g045-sysc.c >>>> @@ -0,0 +1,31 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * RZ/G3S System controller driver >>>> + * >>>> + * Copyright (C) 2024 Renesas Electronics Corp. >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +#include "rz-sysc.h" >>>> + >>>> +#define SYS_USB_PWRRDY 0xd70 >>>> +#define SYS_USB_PWRRDY_PWRRDY_N BIT(0) >>>> +#define SYS_MAX_REG 0xe20 >>>> + >>>> +static const struct rz_sysc_signal_init_data rzg3s_sysc_signals_init_data[] __initconst = { >>> >>> This is marked __initconst... >>> >>>> + { >>>> + .name = "usb-pwrrdy", >>>> + .offset = SYS_USB_PWRRDY, >>>> + .mask = SYS_USB_PWRRDY_PWRRDY_N, >>>> + .refcnt_incr_val = 0 >>>> + } >>>> +}; >>>> + >>>> +const struct rz_sysc_init_data rzg3s_sysc_init_data = { >>> >>> ... but this is not __init, causing a section mismatch. >> >> Do you know if there is a way to detect this? > > The kernel should tell you during the build... I'll look carefully, I haven't noticed it. Thank you! > >> >>> >>>> + .signals_init_data = rzg3s_sysc_signals_init_data, >>>> + .num_signals = ARRAY_SIZE(rzg3s_sysc_signals_init_data), >>>> + .max_register_offset = SYS_MAX_REG, >>>> +}; >>> >>>> --- /dev/null >>>> +++ b/drivers/soc/renesas/rz-sysc.c >>> >>>> +/** >>>> + * struct rz_sysc - RZ SYSC private data structure >>>> + * @base: SYSC base address >>>> + * @dev: SYSC device pointer >>>> + * @signals: SYSC signals >>>> + * @num_signals: number of SYSC signals >>>> + */ >>>> +struct rz_sysc { >>>> + void __iomem *base; >>>> + struct device *dev; >>>> + struct rz_sysc_signal *signals; >>>> + u8 num_signals; >>> >>> You could change signals to a flexible array at the end, tag it with >>> __counted_by(num_signals), and allocate space for both struct rz_sysc >>> and the signals array using struct_size(), reducing the number of >>> allocations. >> >> I'll look into this. > >>>> --- /dev/null >>>> +++ b/drivers/soc/renesas/rz-sysc.h >>>> @@ -0,0 +1,52 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/* >>>> + * Renesas RZ System Controller >>>> + * >>>> + * Copyright (C) 2024 Renesas Electronics Corp. >>>> + */ >>>> + >>>> +#ifndef __SOC_RENESAS_RZ_SYSC_H__ >>>> +#define __SOC_RENESAS_RZ_SYSC_H__ >>>> + >>>> +#include >>>> +#include >>>> + >>>> +/** >>>> + * struct rz_sysc_signal_init_data - RZ SYSC signals init data >>>> + * @name: signal name >>>> + * @offset: register offset controling this signal >>>> + * @mask: bitmask in register specific to this signal >>>> + * @refcnt_incr_val: increment refcnt when setting this value >>>> + */ >>>> +struct rz_sysc_signal_init_data { >>>> + const char *name; >>>> + u32 offset; >>>> + u32 mask; >>>> + u32 refcnt_incr_val; >>>> +}; >>>> + >>>> +/** >>>> + * struct rz_sysc_signal - RZ SYSC signals >>>> + * @init_data: signals initialization data >>>> + * @refcnt: reference counter >>>> + */ >>>> +struct rz_sysc_signal { >>>> + const struct rz_sysc_signal_init_data *init_data; >>> >>> Can't you just embed struct rz_sysc_signal_init_data? >> >> Meaning to have directly the members of struct rz_sysc_signal_init_data >> here or to drop the const qualifier along with __initconst on >> rzg3s_sysc_signals_init_data[] and re-use the platfom data w/o allocate >> new memory? > > I mean > > struct rz_sysc_signal { > struct rz_sysc_signal_init_data init_data; > ... > }; > > Currently you allocate rz_sysc_signal_init_data separately. > When embedded, it will be part of rz_sysc, cfr. above. Ah, your right. I initially had this as a pointer and re-used the init data (rzg3s_sysc_signals_init_data[], w/o having __initconst qualifier for it). I dropped that approach but missed to drop the pointer here. Thank you, Claudiu > >>> That way you could allocate the rz_sysc_signal and >>> rz_sysc_signal_init_data structures in a single allocation. > > Gr{oetje,eeting}s, > > Geert >