Received-SPF: pass (google.com: domain of linux-kernel+bounces-94345-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 6 Mar 2024 12:45:08 -0500
From: Frank Li <Frank.li@nxp.com>
To: Joshua Yeong <joshua.yeong@starfivetech.com>
Cc: "alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	"conor.culhane@silvaco.com" <conor.culhane@silvaco.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"ilpo.jarvinen@linux.intel.com" <ilpo.jarvinen@linux.intel.com>,
	"imx@lists.linux.dev" <imx@lists.linux.dev>,
	"jirislaby@kernel.org" <jirislaby@kernel.org>,
	"joe@perches.com" <joe@perches.com>,
	"krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>,
	"krzysztof.kozlowski@linaro.org" <krzysztof.kozlowski@linaro.org>,
	"linux-i3c@lists.infradead.org" <linux-i3c@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	"miquel.raynal@bootlin.com" <miquel.raynal@bootlin.com>,
	"robh@kernel.org" <robh@kernel.org>,
	"zbigniew.lukwinski@linux.intel.com" <zbigniew.lukwinski@linux.intel.com>
Subject: Re: [PATCH v7 5/8] i3c: target: add svc target controller support
Message-ID: <ZeirpIpcsPuBAiIR@lizhi-Precision-Tower-5810>
References: <20240205233326.552576-1-Frank.Li@nxp.com>
 <20240205233326.552576-6-Frank.Li@nxp.com>
 <SH0PR01MB08410A70ABB956DF9DD4DE25F921A@SH0PR01MB0841.CHNPR01.prod.partner.outlook.cn>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <SH0PR01MB08410A70ABB956DF9DD4DE25F921A@SH0PR01MB0841.CHNPR01.prod.partner.outlook.cn>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?w2IZFBL+jLBiZoDB1hIHlyDt3Pkecr7SUSOTsBeGKhIQvo3PDtqxaKRK3M4C?=
 =?us-ascii?Q?r9gfM/myjwBxzGhLuW3DlSN9qAV4G+8x79FsP5AwS0jH2rmh37B2Expe4FXs?=
 =?us-ascii?Q?RnLMZXoTWjbIQk/AE1B4PNWGDqplvucdQ41lsD3OveizDJfFMDBJS18tz/WO?=
 =?us-ascii?Q?dCat6+exOwKBG6cXLDz7NRKb3SPlUkkGbVWXK8bS5WlZEe3d5ZFzOGbSKpYN?=
 =?us-ascii?Q?hwtXjtx3uQp6TunJmHX9E+suTrxZIaOstl87Zmw/7GiEAvFat68+7Xkbr3am?=
 =?us-ascii?Q?G0us8LTSRrjSJnHKDZQndk1MKzO6J6Sktie7MDcBsfgN0U65YfPVQexcecbE?=
 =?us-ascii?Q?nDd29Z55/tQQG/UsqeUK82CtvkPD4fO2T3L301rMsV0lNxybRQ2QD61MM96v?=
 =?us-ascii?Q?xhLmfNERc6i8dgR9hK5WNJNLRb3X8hI+pDSGU8KT3f/xpglL7sOAkIF8ufyY?=
 =?us-ascii?Q?RyPSZukROhQzn79yZN0PY4uU0Yz34rryMCVhfs9QEp9LWQLfuTfO0HHW+38y?=
 =?us-ascii?Q?7el1cWsUUpd0CmylzpyhNujYHH+yPnA3PiPKKX66TBsaP6Mv7FqNWsrUwPlz?=
 =?us-ascii?Q?qsDujrqOMH0SSakotlUbmzWo2FgBe1iB2RyCDUucvnDotMGpAACXkpwihYmP?=
 =?us-ascii?Q?9jOv5Gws/S1+bwzW30mQARsCWzBvdkmRMqHu8Z5NQWQotkPDJBFfaR9gzPJ0?=
 =?us-ascii?Q?K6rc9XdAaLFhez7459aX7y5TD0SELsJ8txaujYb/n9a7zJw8Q6Q3eGo1bZtt?=
 =?us-ascii?Q?Z6yRBzoaK2jtEhvCIU9RmIl3lUrNjWTLeG5l9/a6QVuOkstJMCIX+mPFKG7k?=
 =?us-ascii?Q?RIXkrhrmaBn6hpBC0TJSIITVd9lerFCCbjqcgB+npnhmufeGI00McrWWX+Yb?=
 =?us-ascii?Q?xjDDnXK/rHrOkVmv9ROBUxEY5pVNKJI2mgfeoh7TXBzaQFusu9NkdrDuI5D7?=
 =?us-ascii?Q?iaBAhuDGf4/BX7YDePGcCRjhXsUxbtMorkNymDWkftFtIH6fa+CHJrWgTUX6?=
 =?us-ascii?Q?/KVRk3BjsjOyZVDgPttMQxdgDnucZ45VFJHmWHZEQjNC1LP0BqakAGy7pStq?=
 =?us-ascii?Q?ftq8EqjsgvfjtcALsU8A5kEiLW/TKcPjBddoe8BVlUU5kQPYCFqF/9kYOrlR?=
 =?us-ascii?Q?WNUKfyNGMVDvgOshr+vMMic/mR+fc21lElSggbHoZ5vGviNhota0lq6x7MYE?=
 =?us-ascii?Q?90l9kBfXzIQijbMd445m2SNdUC7QhywkKQtvqwiRo1//dLZihS13Ze+hijmm?=
 =?us-ascii?Q?blcjxN4Hnjvew8VN3XT3tj3ntl9ov7zXp4OEx/dtO5+1vHrqQTsyN0pycMzI?=
 =?us-ascii?Q?PA2l1reW6epCWE6AX18WpggBej3EvawsTs6hvieSjzULSgyM56aQSWE6lfsr?=
 =?us-ascii?Q?Z/FK0hBXQYqlC1vmQS1HXOG/HGdUyUf14GIKn0eZhXjgnT5WILTUrdOx51el?=
 =?us-ascii?Q?FTq7+Ef7oFfkN0jEoCJGsPhfnPrprF03sgUZ3PdX4jqMteAGvInp6dR68+Oh?=
 =?us-ascii?Q?jhcCFRK+Ux/CHF1jZvIGs+58xNVTBIrEVAnB2NCMlCAyNmhowmmz37STdPEt?=
 =?us-ascii?Q?I++aO3khKZwbB4rEqzqmjvOxj+GTiuoZlawXiH6F?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f1b7c992-98b2-41d7-a336-08dc3e052f97
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB9642.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2024 17:45:17.6114
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: eG9S7bqeGBh9sPjheOysWrrKYjbGWyJc3kfmGG2zvjsKF+nUyNq5k5cr5hkF63AqbAy+z3mslQ8c1VRxoZGgVQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PR3PR04MB7243

On Wed, Mar 06, 2024 at 04:01:17PM +0000, Joshua Yeong wrote:
> Hi Frank,
> 
> > -----Original Message-----
> > From: linux-i3c <linux-i3c-bounces@lists.infradead.org> On Behalf Of Frank Li
> > Sent: Tuesday, February 6, 2024 7:33 AM
> > To: frank.li@nxp.com
> > Cc: alexandre.belloni@bootlin.com; conor.culhane@silvaco.com;
> > devicetree@vger.kernel.org; gregkh@linuxfoundation.org;
> > ilpo.jarvinen@linux.intel.com; imx@lists.linux.dev; jirislaby@kernel.org;
> > joe@perches.com; krzysztof.kozlowski+dt@linaro.org;
> > krzysztof.kozlowski@linaro.org; linux-i3c@lists.infradead.org; linux-
> > kernel@vger.kernel.org; linux-serial@vger.kernel.org;
> > miquel.raynal@bootlin.com; robh@kernel.org;
> > zbigniew.lukwinski@linux.intel.com
> > Subject: [PATCH v7 5/8] i3c: target: add svc target controller support
> > 
> > Add Silvaco I3C target controller support
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > 
> > Notes:
> >     Change from v2 to v3
> >     - fix build warning
> > 
> >  drivers/i3c/master/Makefile         |   2 +-
> >  drivers/i3c/master/svc-i3c-main.c   |  35 +-
> >  drivers/i3c/master/svc-i3c-target.c | 776
> 
> I think putting target mode files under "master" might not make sense. We might have to
> consider that we may have a "secondary master" mode. Some other ways of splitting
> or handling of target mode is needed here.

I think name 'master' is not good here. Previously only support 'master'
mode, it should be fine. Now many controller are dual mode.

And I3C spec use 'controller/target' instead of 'master/slave'. I think
'controller' as master are quite confused. It can be master controller
and slave controller.

Anyway, slave/master may share some code and resource, even only one file.

So far, I think it is fine put under master now. we can rename 'master'
later when more dual mode controller added. 

Frank 

> 
> ...
> 
> > +
> > +#define I3C_SCONFIG	0x4
> > +#define   I3C_SCONFIG_SLVENA_MASK	BIT(0)
> > +#define	  I3C_SCONFIG_OFFLINE_MASK	BIT(9)
> > +#define   I3C_SCONFIG_SADDR_MASK	GENMASK(31, 25)
> > +
> > +#define I3C_SSTATUS	0x8
> > +#define	  I3C_SSTATUS_STNOTSTOP_MASK	BIT(0)
> > +#define	  I3C_SSTATUS_STOP_MASK		BIT(10)
> > +#define	  I3C_SSTATUS_RX_PEND_MASK	BIT(11)
> > +#define   I3C_SSTATUS_TXNOTFULL_MASK	BIT(12)
> > +#define	  I3C_SSTATUS_DACHG_MASK	BIT(13)
> > +#define	  I3C_SSTATUS_EVDET_MASK	GENMASK(21, 20)
> > +#define	  I3C_SSTATUS_EVDET_ACKED	0x3
> > +#define	  I3C_SSTATUS_IBIDIS_MASK	BIT(24)
> > +#define	  I3C_SSTATUS_HJDIS_MASK	BIT(27)
> > +
> 
> There is couple of space formatting here that requires to be fixed.
> 
> Cheers,
> Joshua