On Mon, Feb 24, 2025 at 04:39:05PM +0800, Stanley Chu wrote: > From: Stanley Chu <yschu@xxxxxxxxxxx> > > Nuvoton npcm845 SoC uses the same Silvico IP but an older version. > Add quirks to address the npcm845 specific issues. It'd better list issue here. > > Signed-off-by: Stanley Chu <yschu@xxxxxxxxxxx> > --- > drivers/i3c/master/svc-i3c-master.c | 56 +++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index d6057d8c7dec..8834f87a4767 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -32,6 +32,7 @@ > #define SVC_I3C_MCONFIG_ODBAUD(x) FIELD_PREP(GENMASK(23, 16), (x)) > #define SVC_I3C_MCONFIG_ODHPP(x) FIELD_PREP(BIT(24), (x)) > #define SVC_I3C_MCONFIG_SKEW(x) FIELD_PREP(GENMASK(27, 25), (x)) > +#define SVC_I3C_MCONFIG_SKEW_MASK GENMASK(27, 25) > #define SVC_I3C_MCONFIG_I2CBAUD(x) FIELD_PREP(GENMASK(31, 28), (x)) > > #define SVC_I3C_MCTRL 0x084 > @@ -133,6 +134,32 @@ > #define SVC_I3C_EVENT_IBI GENMASK(7, 0) > #define SVC_I3C_EVENT_HOTJOIN BIT(31) > > +/* > + * SVC_I3C_QUIRK_FIFO_EMPTY: > + * I3C HW stalls the write transfer if the transmit FIFO becomes empty, > + * when new data is written to FIFO, I3C HW resumes the transfer but > + * the first transmitted data bit may have the wrong value. > + * Workaround: > + * Fill the FIFO in advance to prevent FIFO from becoming empty. > + */ > +#define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) > +/* > + * SVC_I3C_QUIRK_FLASE_SLVSTART: > + * I3C HW may generate an invalid SlvStart event when emitting a STOP. > + * If it is a true SlvStart, the MSTATUS state is SLVREQ. > + */ > +#define SVC_I3C_QUIRK_FALSE_SLVSTART BIT(1) > +/* > + * SVC_I3C_QUIRK_DAA_CORRUPT: > + * When MCONFIG.SKEW=0 and MCONFIG.ODHPP=0, the ENTDAA transaction gets > + * corrupted and results in a no repeated-start condition at the end of > + * address assignment. > + * Workaround: > + * Set MCONFIG.SKEW to 1 before initiating the DAA process. After the DAA > + * process is completed, return MCONFIG.SKEW to its previous value. > + */ > +#define SVC_I3C_QUIRK_DAA_CORRUPT BIT(2) > + > struct svc_i3c_cmd { > u8 addr; > bool rnw; > @@ -158,6 +185,10 @@ struct svc_i3c_regs_save { > u32 mdynaddr; > }; > > +struct svc_i3c_drvdata { > + u32 quirks; > +}; > + > /** > * struct svc_i3c_master - Silvaco I3C Master structure > * @base: I3C master controller > @@ -183,6 +214,7 @@ struct svc_i3c_regs_save { > * @ibi.tbq_slot: To be queued IBI slot > * @ibi.lock: IBI lock > * @lock: Transfer lock, protect between IBI work thread and callbacks from master > + * @drvdata: Driver data > * @enabled_events: Bit masks for enable events (IBI, HotJoin). > * @mctrl_config: Configuration value in SVC_I3C_MCTRL for setting speed back. > */ > @@ -214,6 +246,7 @@ struct svc_i3c_master { > spinlock_t lock; > } ibi; > struct mutex lock; > + const struct svc_i3c_drvdata *drvdata; > u32 enabled_events; > u32 mctrl_config; > }; > @@ -230,6 +263,27 @@ struct svc_i3c_i2c_dev_data { > struct i3c_generic_ibi_pool *ibi_pool; > }; > > +const struct svc_i3c_drvdata npcm845_drvdata = { > + .quirks = SVC_I3C_QUIRK_FIFO_EMPTY | SVC_I3C_QUIRK_FALSE_SLVSTART > + | SVC_I3C_QUIRK_DAA_CORRUPT, > +}; > + > +static bool svc_has_quirk(struct svc_i3c_master *master, u32 quirk) > +{ > + if (!master->drvdata) > + return false; You's better add drvdata for exist one to keep consistent for all compatibe string. > + > + if ((master->drvdata->quirks & quirk) == SVC_I3C_QUIRK_DAA_CORRUPT) { > + if (master->mctrl_config & > + (SVC_I3C_MCONFIG_SKEW_MASK | SVC_I3C_MCONFIG_ODHPP(1))) > + return false; This helper function should only check quirk, you addtional check for mctrl_config, which exceed this function scope. Or you add new helper funciton check daa_corrupt only, such as svs_has_daa_corrupt() ? > + else > + return true; > + } > + > + return (master->drvdata->quirks & quirk); > +} > + > static inline bool is_events_enabled(struct svc_i3c_master *master, u32 mask) > { > return !!(master->enabled_events & mask); > @@ -1868,6 +1922,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev) > } > > platform_set_drvdata(pdev, master); > + master->drvdata = of_device_get_match_data(dev); > > pm_runtime_set_autosuspend_delay(&pdev->dev, SVC_I3C_PM_TIMEOUT_MS); > pm_runtime_use_autosuspend(&pdev->dev); > @@ -1960,6 +2015,7 @@ static const struct dev_pm_ops svc_i3c_pm_ops = { > > static const struct of_device_id svc_i3c_master_of_match_tbl[] = { > { .compatible = "silvaco,i3c-master-v1"}, silcaco_default_drvdata = {}; .compatible = &silcaco_default_drvdata; > + { .compatible = "nuvoton,npcm845-i3c", .data = &npcm845_drvdata }, > { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl); > -- > 2.34.1 >