On Wed, Mar 06, 2024 at 04:01:17PM +0000, Joshua Yeong wrote: > Hi Frank, > > > -----Original Message----- > > From: linux-i3c <linux-i3c-bounces@xxxxxxxxxxxxxxxxxxx> On Behalf Of Frank Li > > Sent: Tuesday, February 6, 2024 7:33 AM > > To: frank.li@xxxxxxx > > Cc: alexandre.belloni@xxxxxxxxxxx; conor.culhane@xxxxxxxxxxx; > > devicetree@xxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; > > ilpo.jarvinen@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; jirislaby@xxxxxxxxxx; > > joe@xxxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; > > krzysztof.kozlowski@xxxxxxxxxx; linux-i3c@xxxxxxxxxxxxxxxxxxx; linux- > > kernel@xxxxxxxxxxxxxxx; linux-serial@xxxxxxxxxxxxxxx; > > miquel.raynal@xxxxxxxxxxx; robh@xxxxxxxxxx; > > zbigniew.lukwinski@xxxxxxxxxxxxxxx > > 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@xxxxxxx> > > --- > > > > 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
