> -----Original Message----- > From: Frank Li <Frank.li@xxxxxxx> > Sent: Thursday, March 7, 2024 1:45 AM > To: Joshua Yeong <joshua.yeong@xxxxxxxxxxxxxxxx> > 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: Re: [PATCH v7 5/8] i3c: target: add svc target controller support > > 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 I am currently working on secondary-controller mode. I would like to explore if we can have a different way to implement various modes. I am guessing that the current inspiration is coming from the existing i2c framework. However, I am thinking of having a more generic function to replace the way how we register the i3c driver. (existing 'i3c_master_register' or 'devm_i3c_target_ctrl_create' from yours) Maybe along the line where we can have all the modes set in a struct and return to the i3c framework. So we only need 1 API to register all the different modes instead of having one each. > > > > > ... > > > > > + > > > +#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
