From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f169.google.com (mail-qk1-f169.google.com [209.85.222.169]) (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 569BF27FB21 for ; Tue, 30 Dec 2025 00:50:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767055816; cv=none; b=nyfpJyjYW3y3pNIZbmm9mP1E1NKhqrOkS2v9b+v74FrSkYMucVm22qy9oIG1ZZphmtDiqfjnu7U1nAIOdK7Yjk0tyxUShq98MOTxvI0WGivXE1vxF1FZcwj4qtaXPfWZWJZsMsAipa13t8kf+Bq7ZTE9BqsB32vooBpUIogZMnw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767055816; c=relaxed/simple; bh=42Du2CLON1ecQ5mNngEuokBJCgYS/KHqsmqoyYaW7kE=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=Vyd2DXzAq7Xgn6FiVMZSVD1AYsK7+pWbA7U36l2jCZDVi+Ad9IduXVb87Uoa7rHp4QfseGwalytXYcLIWguXMj5DCEkm4rKiqYUF5u+PdXJqwyiTgmGgUcPCmUuw/saY4mzJMEogQVCHbB4hSwUFsk9RjPbiZtA4PZQdF85do78= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=riscstar.com; spf=pass smtp.mailfrom=riscstar.com; dkim=pass (2048-bit key) header.d=riscstar-com.20230601.gappssmtp.com header.i=@riscstar-com.20230601.gappssmtp.com header.b=WBm0aAc2; arc=none smtp.client-ip=209.85.222.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=riscstar.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=riscstar.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=riscstar-com.20230601.gappssmtp.com header.i=@riscstar-com.20230601.gappssmtp.com header.b="WBm0aAc2" Received: by mail-qk1-f169.google.com with SMTP id af79cd13be357-8b2f0f9e4cbso711246285a.0 for ; Mon, 29 Dec 2025 16:50:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=riscstar-com.20230601.gappssmtp.com; s=20230601; t=1767055813; x=1767660613; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=NXi9h2F6u/+Ag/dMKJ4V4r8/OUgCm6fykDTUehBQdYg=; b=WBm0aAc2WOPomHLDHBQQK+pRUaq4wUn48KZX2+F4W37FcjAEfko/uaybV+8waLKcfW VJ5YjzpBPr/2rEn4/eTEUHGzMhnM9l1ouIOD0029CvJ2wjzNKyj14NmxNqK8MccaWHPN e110l+BIIbIC4hu9nz3qGu3HrLkhJots8NwNxTOU8G2exylB9ArPfKJN2Uzscr1cKTNS wFbEqsKry29vDnmYsRmJsWcc+SL7vv2b17VQvAaJC8KGiTC89FwXgWi95GkeonNUJ9wP J44AbsrpKwnCvbQpiL+nbabaxAs36auSWukZX16RnUhUemyViSBjpkUcAq+ieNEasT29 UrRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767055813; x=1767660613; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NXi9h2F6u/+Ag/dMKJ4V4r8/OUgCm6fykDTUehBQdYg=; b=eTZOiESkVhv9rEO/k+y5ymSydJ9Zu75B2uyEXZ5LY7Oxf369B/jPIpmk9NNvHwUG8Q fXNrl+GrnpTypzJlms4GYD4SBMgsAnYi6FFL4UdfW/87jhhuoB9cvRqyfVCr8CB+sCUB sSHnImdlqa8pANMvRbwZBe7MujDNm54xJ4qiZh6+eHCSX7ntB1bmoynYL6OUbAe+6Jwd iAEWcuq++UHGxMXbawk0RVvUO9pdjv+A7ujWLfLlxoeV2k6HMSkhgVdiYVYQ0HQMPBfS Bepwnnqh5hFWmTs/loIV4KvA8hfiBzkIuVqoVLQt6/z8N/tpFq6bXB0YD2iLAd1a/GUe 4/Vw== X-Forwarded-Encrypted: i=1; AJvYcCW3wLi7e+aDiHNUrH9Y7+jMoW6XxRsAV0YuqH7koskjWUnvdkRTmpcxT+MHyaW042PIdAuGDLaXAf0fzZk=@vger.kernel.org X-Gm-Message-State: AOJu0Yz+sCiwGCp8YqdVgP/FJ25M2KKSwJl33VLPqMP4bc30XosZQNWP AIOZJvb90EmXE4rGmzo1isidz7hVOmrYltjWWNgPas+z5sZEyoUuRxNMvLBq+ncZ5rs= X-Gm-Gg: AY/fxX5hCyFw5i1vSj0x7QROfXV9ypgOqoZzpuAiLMeVxs+IfXIGFYZNdHK3I6G/mtK C1/EwLFhE6VYD8SLr2cH52Km8fEUJeKLiiJgzXlTnVgAYg5/nxW63Jqv80K7KRymZrYvqyjRUl8 lVrBghncEDUj9eSA88HZzMYnT5Rmprx006R2l8ZJ2HBoWPiJz47fEDaEuk4jMezq8o9j+/MFql7 ik+xokTQiRBRzPCbWxiC5mHyBspYq+kPNE6dQv3goEzsVl2DE007i9lhayHtVbDDMgnzdG9OiKL v+FndbclHgFx3szAapmOEtTkQt/bsPWMlyEwvhWkcvA8+rgwq/9e6B3tLq6p76IS99wU0OxdZgo WN4VU4RiPdvJtHGmmcXy4lmpI6aR4mcFadMOz0t7u0l3ODM6Z3GzyfYclYU/RDNZ0uM1njmrSLS +5P3O6KTBxP1rkP/v2Y0Bl04NougUzl2qImCy4ThStiblspaaVtEOG+Ri6l+GKkA== X-Google-Smtp-Source: AGHT+IHk1jnBQLry143qcwBHNuIgjGX/ECeDXUHHBnJflR5W+2APDyw+OrUHlSdhTu9nzhHG4A7//Q== X-Received: by 2002:a05:620a:3f85:b0:8a2:595f:657 with SMTP id af79cd13be357-8bee76c8eb7mr5352874385a.21.1767055813193; Mon, 29 Dec 2025 16:50:13 -0800 (PST) Received: from [172.22.22.28] (c-75-72-117-212.hsd1.mn.comcast.net. [75.72.117.212]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8c096891558sm2482817485a.15.2025.12.29.16.50.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 29 Dec 2025 16:50:12 -0800 (PST) Message-ID: <17c27455-897d-4249-8206-88364230af7d@riscstar.com> Date: Mon, 29 Dec 2025 18:50:11 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Alex Elder Subject: Re: [PATCH v2 1/3] clk: spacemit: prepare common ccu header To: Yixun Lan , Stephen Boyd , Michael Turquette , Philipp Zabel Cc: Guodong Xu , Inochi Amaoto , linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-riscv@lists.infradead.org, spacemit@lists.linux.dev References: <20251226-06-k1-clk-common-v2-0-28b59418b4df@gentoo.org> <20251226-06-k1-clk-common-v2-1-28b59418b4df@gentoo.org> Content-Language: en-US In-Reply-To: <20251226-06-k1-clk-common-v2-1-28b59418b4df@gentoo.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 12/26/25 12:55 AM, Yixun Lan wrote: > In order to prepare adding clock driver for new SoC, extract common > ccu header file, so it can be shared by all drivers. You are moving the definition of the SpacemiT CCU auxiliary device structure, plus the to_spacemit_ccu_adev() function, into a new header file. The reason you're doing this is because these two things are generic, but they're defined in the K1 SoC-specific header file "k1-syscon.h". So you are creating a new header file for this purpose. These are things you should explain here, to help orient reviewers and will inform anyone in the future looking at commit history. > Also introduce a reset name macro, so it can be both used in clock > and reset subsystem, explicitly to make them match each other. This should go in a separate patch, and should change the code to use the macro so it builds and continues to function with the new change place. However I don't understand why you think it's necessary to introduce the reset name macro. Is it because you want to incorporate an SoC identifier in the name? Even if this is your reason, I still don't think you need the macro. I'll try to explain what I mean in the next patch. One more comment, below. > Signed-off-by: Yixun Lan > --- > include/soc/spacemit/ccu.h | 21 +++++++++++++++++++++ > include/soc/spacemit/k1-syscon.h | 13 +++---------- > 2 files changed, 24 insertions(+), 10 deletions(-) > > diff --git a/include/soc/spacemit/ccu.h b/include/soc/spacemit/ccu.h > new file mode 100644 > index 000000000000..84dcdecccc05 > --- /dev/null > +++ b/include/soc/spacemit/ccu.h > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#ifndef __SOC_SPACEMIT_CCU_H__ > +#define __SOC_SPACEMIT_CCU_H__ > + > +#include > +#include > + > +/* Auxiliary device used to represent a CCU reset controller */ > +struct spacemit_ccu_adev { > + struct auxiliary_device adev; > + struct regmap *regmap; > +}; > + > +static inline struct spacemit_ccu_adev * > +to_spacemit_ccu_adev(struct auxiliary_device *adev) > +{ > + return container_of(adev, struct spacemit_ccu_adev, adev); > +} > + > +#endif /* __SOC_SPACEMIT_CCU_H__ */ > diff --git a/include/soc/spacemit/k1-syscon.h b/include/soc/spacemit/k1-syscon.h > index 354751562c55..13efa7a30853 100644 > --- a/include/soc/spacemit/k1-syscon.h > +++ b/include/soc/spacemit/k1-syscon.h > @@ -5,17 +5,10 @@ > #ifndef __SOC_K1_SYSCON_H__ > #define __SOC_K1_SYSCON_H__ > > -/* Auxiliary device used to represent a CCU reset controller */ > -struct spacemit_ccu_adev { > - struct auxiliary_device adev; > - struct regmap *regmap; > -}; > +#include "ccu.h" > > -static inline struct spacemit_ccu_adev * > -to_spacemit_ccu_adev(struct auxiliary_device *adev) > -{ > - return container_of(adev, struct spacemit_ccu_adev, adev); > -} > +/* Reset name macro, should match in clock and reset */ > +#define _K_RST(_unit) "k1-" #_unit "-reset" The generic-sounding _K_RST() encodes "k1" in the name, and it shouldn't. Also, why do you use the underscore prefix? Anyway, I'll keep reading. -Alex > > /* APBS register offset */ > #define APBS_PLL1_SWCR1 0x100 >