lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z8CcseC8bLefDVG6@lizhi-Precision-Tower-5810>
Date: Thu, 27 Feb 2025 12:11:13 -0500
From: Frank Li <Frank.li@....com>
To: Stanley Chu <stanley.chuys@...il.com>
Cc: miquel.raynal@...tlin.com, alexandre.belloni@...tlin.com,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, tomer.maimon@...oton.com,
	kwliu@...oton.com, yschu@...oton.com
Subject: Re: [PATCH v5 2/5] i3c: master: svc: Add support for Nuvoton npcm845
 i3c

On Thu, Feb 27, 2025 at 02:01:28PM +0800, Stanley Chu wrote:
> From: Stanley Chu <yschu@...oton.com>
>
> Nuvoton npcm845 SoC uses the same Silvico IP but an older version.
> Add quirks to address the npcm845 specific issues.
>
> FIFO empty issue:
> 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.
>
> Invalid SlvStart event:
> I3C HW may generate an invalid SlvStart event when emitting a STOP.
>
> DAA process corruption:
> 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.
>
> Signed-off-by: Stanley Chu <yschu@...oton.com>
> ---
>  drivers/i3c/master/svc-i3c-master.c | 56 ++++++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index d6057d8c7dec..9143a079de53 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)
> +

Generally define QUIRK when it was real used. I suggest his patch just add
struct svc_i3c_drvdata {
	u32 quirks.
}
And svc_default_drvdata and svc_default_drvdata. commit message said

Add quirks for difference compatible string and pave the road to support
Nuvoton.

If alex think that define here is good, I am fine for this.

Frank

>  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,25 @@ struct svc_i3c_i2c_dev_data {
>  	struct i3c_generic_ibi_pool *ibi_pool;
>  };
>
> +const struct svc_i3c_drvdata svc_default_drvdata = {};
> +const struct svc_i3c_drvdata npcm845_drvdata = {
> +	.quirks = SVC_I3C_QUIRK_FIFO_EMPTY |
> +		SVC_I3C_QUIRK_FALSE_SLVSTART |
> +		SVC_I3C_QUIRK_DAA_CORRUPT,
> +};
> +
> +static inline bool svc_has_quirk(struct svc_i3c_master *master, u32 quirk)
> +{
> +	return (master->drvdata->quirks & quirk);
> +}
> +
> +static inline bool svc_has_daa_corrupt(struct svc_i3c_master *master)
> +{
> +	return ((master->drvdata->quirks & SVC_I3C_QUIRK_DAA_CORRUPT) &&
> +		!(master->mctrl_config &
> +		(SVC_I3C_MCONFIG_SKEW_MASK | SVC_I3C_MCONFIG_ODHPP(1))));
> +}
> +
>  static inline bool is_events_enabled(struct svc_i3c_master *master, u32 mask)
>  {
>  	return !!(master->enabled_events & mask);
> @@ -1868,6 +1920,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);
> @@ -1959,7 +2012,8 @@ 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"},
> +	{ .compatible = "silvaco,i3c-master-v1", .data = &svc_default_drvdata },
> +	{ .compatible = "nuvoton,npcm845-i3c", .data = &npcm845_drvdata },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl);
> --
> 2.34.1
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ