[PATCH 00/13] i3c: mipi-i3c-hci-pci: Add LTR support for Intel controllers

[PATCH 00/13] i3c: mipi-i3c-hci-pci: Add LTR support for Intel controllers

[Adrian Hunter]

Hi

Here are patches primarily to add support for Latency Tolerance Reporting
(LTR) for Intel controllers.  Most patches are small tidy-ups or
preparation for the main change.


Adrian Hunter (13):
      i3c: mipi-i3c-hci-pci: Set 64-bit DMA mask for Intel controllers
      i3c: mipi-i3c-hci-pci: Move all Intel-related definitions together
      i3c: mipi-i3c-hci-pci: Rename some Intel-related identifiers
      i3c: mipi-i3c-hci-pci: Use readl_poll_timeout()
      i3c: mipi-i3c-hci-pci: Constify driver data
      i3c: mipi-i3c-hci-pci: Factor out private registers ioremapping
      i3c: mipi-i3c-hci-pci: Factor out intel_reset()
      i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information
      i3c: mipi-i3c-hci-pci: Change callback parameter
      i3c: mipi-i3c-hci-pci: Add exit callback
      i3c: mipi-i3c-hci-pci: Allocate a structure for Intel controller information
      i3c: mipi-i3c-hci-pci: Add LTR support for Intel controllers
      i3c: mipi-i3c-hci-pci: Set d3cold_delay to 0 for Intel controllers

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 236 +++++++++++++++++----
 1 file changed, 198 insertions(+), 38 deletions(-)


Regards
Adrian

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 01/13] i3c: mipi-i3c-hci-pci: Set 64-bit DMA mask for Intel controllers

[Adrian Hunter]

All Intel controllers support 64-bit DMA.  Set the DMA mask accordingly.

This is needed only if there is no IOMMU configured, which is not
recommended, but possible.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index dd4484eff2f0..850fcec4cacf 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -29,6 +29,7 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
 {
 	unsigned long timeout;
 	void __iomem *priv;
+	int ret;
 
 	priv = devm_ioremap(&pci->dev,
 			    pci_resource_start(pci, 0) + INTEL_PRIV_OFFSET,
@@ -36,6 +37,10 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
 	if (!priv)
 		return -ENOMEM;
 
+	ret = dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(64));
+	if (ret)
+		return ret;
+
 	/* Assert reset, wait for completion and release reset */
 	writel(0, priv + INTEL_PRIV_RESETS);
 	timeout = jiffies + msecs_to_jiffies(10);
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 01/13] i3c: mipi-i3c-hci-pci: Set 64-bit DMA mask for Intel controllers

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:27PM +0200, Adrian Hunter wrote:
> All Intel controllers support 64-bit DMA.  Set the DMA mask accordingly.
>
> This is needed only if there is no IOMMU configured, which is not
> recommended, but possible.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index dd4484eff2f0..850fcec4cacf 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -29,6 +29,7 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
>  {
>  	unsigned long timeout;
>  	void __iomem *priv;
> +	int ret;
>
>  	priv = devm_ioremap(&pci->dev,
>  			    pci_resource_start(pci, 0) + INTEL_PRIV_OFFSET,
> @@ -36,6 +37,10 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
>  	if (!priv)
>  		return -ENOMEM;
>
> +	ret = dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(64));
> +	if (ret)
> +		return ret;
>

Needn't check return value dma_set_mask_and_coherent(), it is always true
if mask >= 32

See Documentation/core-api/dma-api-howto.rst

commit f7ae20f2fc4e6a5e32f43c4fa2acab3281a61c81
Author: Frank Li <Frank.Li@nxp.com>
Date:   Mon Apr 1 13:41:59 2024 -0400

    docs: dma: correct dma_set_mask() sample code


Frank
>  	/* Assert reset, wait for completion and release reset */
>  	writel(0, priv + INTEL_PRIV_RESETS);
>  	timeout = jiffies + msecs_to_jiffies(10);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 02/13] i3c: mipi-i3c-hci-pci: Move all Intel-related definitions together

[Adrian Hunter]

Move all Intel-related definitions together, to tidy the code slightly.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 850fcec4cacf..617826628dc4 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -17,14 +17,14 @@ struct mipi_i3c_hci_pci_info {
 	int (*init)(struct pci_dev *pci);
 };
 
+static DEFINE_IDA(mipi_i3c_hci_pci_ida);
+
 #define INTEL_PRIV_OFFSET		0x2b0
 #define INTEL_PRIV_SIZE			0x28
 #define INTEL_PRIV_RESETS		0x04
 #define INTEL_PRIV_RESETS_RESET		BIT(0)
 #define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
 
-static DEFINE_IDA(mipi_i3c_hci_pci_ida);
-
 static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
 {
 	unsigned long timeout;
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 02/13] i3c: mipi-i3c-hci-pci: Move all Intel-related definitions together

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:28PM +0200, Adrian Hunter wrote:
> Move all Intel-related definitions together, to tidy the code slightly.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

looks like not neccessary.

Frank

>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 850fcec4cacf..617826628dc4 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -17,14 +17,14 @@ struct mipi_i3c_hci_pci_info {
>  	int (*init)(struct pci_dev *pci);
>  };
>
> +static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> +
>  #define INTEL_PRIV_OFFSET		0x2b0
>  #define INTEL_PRIV_SIZE			0x28
>  #define INTEL_PRIV_RESETS		0x04
>  #define INTEL_PRIV_RESETS_RESET		BIT(0)
>  #define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
>
> -static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> -
>  static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
>  {
>  	unsigned long timeout;
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 02/13] i3c: mipi-i3c-hci-pci: Move all Intel-related definitions together

[Adrian Hunter]

On 12/11/2025 17:53, Frank Li wrote:
> On Wed, Nov 12, 2025 at 12:03:28PM +0200, Adrian Hunter wrote:
>> Move all Intel-related definitions together, to tidy the code slightly.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
> 
> looks like not neccessary.

Sure, it is just a tidy-up.

It is confusing to have something non-Intel related in the middle
of all the Intel definitions.

> 
> Frank
> 
>>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> index 850fcec4cacf..617826628dc4 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> @@ -17,14 +17,14 @@ struct mipi_i3c_hci_pci_info {
>>  	int (*init)(struct pci_dev *pci);
>>  };
>>
>> +static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>> +
>>  #define INTEL_PRIV_OFFSET		0x2b0
>>  #define INTEL_PRIV_SIZE			0x28
>>  #define INTEL_PRIV_RESETS		0x04
>>  #define INTEL_PRIV_RESETS_RESET		BIT(0)
>>  #define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
>>
>> -static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>> -
>>  static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
>>  {
>>  	unsigned long timeout;
>> --
>> 2.51.0
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 03/13] i3c: mipi-i3c-hci-pci: Rename some Intel-related identifiers

[Adrian Hunter]

Rename some Intel-related identifiers to ensure Intel-related identifiers
begin "intel_" or INTEL_, and for brevity.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 617826628dc4..c74cc511a344 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -21,11 +21,11 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
 
 #define INTEL_PRIV_OFFSET		0x2b0
 #define INTEL_PRIV_SIZE			0x28
-#define INTEL_PRIV_RESETS		0x04
-#define INTEL_PRIV_RESETS_RESET		BIT(0)
-#define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
+#define INTEL_RESETS			0x04
+#define INTEL_RESETS_RESET		BIT(0)
+#define INTEL_RESETS_RESET_DONE		BIT(1)
 
-static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
+static int intel_init(struct pci_dev *pci)
 {
 	unsigned long timeout;
 	void __iomem *priv;
@@ -42,21 +42,21 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
 		return ret;
 
 	/* Assert reset, wait for completion and release reset */
-	writel(0, priv + INTEL_PRIV_RESETS);
+	writel(0, priv + INTEL_RESETS);
 	timeout = jiffies + msecs_to_jiffies(10);
-	while (!(readl(priv + INTEL_PRIV_RESETS) &
-		 INTEL_PRIV_RESETS_RESET_DONE)) {
+	while (!(readl(priv + INTEL_RESETS) &
+		 INTEL_RESETS_RESET_DONE)) {
 		if (time_after(jiffies, timeout))
 			break;
 		cpu_relax();
 	}
-	writel(INTEL_PRIV_RESETS_RESET, priv + INTEL_PRIV_RESETS);
+	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
 
 	return 0;
 }
 
 static struct mipi_i3c_hci_pci_info intel_info = {
-	.init = mipi_i3c_hci_pci_intel_init,
+	.init = intel_init,
 };
 
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 03/13] i3c: mipi-i3c-hci-pci: Rename some Intel-related identifiers

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:29PM +0200, Adrian Hunter wrote:
> Rename some Intel-related identifiers to ensure Intel-related identifiers
> begin "intel_" or INTEL_, and for brevity.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 617826628dc4..c74cc511a344 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -21,11 +21,11 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>
>  #define INTEL_PRIV_OFFSET		0x2b0
>  #define INTEL_PRIV_SIZE			0x28
> -#define INTEL_PRIV_RESETS		0x04
> -#define INTEL_PRIV_RESETS_RESET		BIT(0)
> -#define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
> +#define INTEL_RESETS			0x04
> +#define INTEL_RESETS_RESET		BIT(0)
> +#define INTEL_RESETS_RESET_DONE		BIT(1)

These are already begin with INTEL.

>
> -static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
> +static int intel_init(struct pci_dev *pci)

intel_i3c_init() ?

Frank

>  {
>  	unsigned long timeout;
>  	void __iomem *priv;
> @@ -42,21 +42,21 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
>  		return ret;
>
>  	/* Assert reset, wait for completion and release reset */
> -	writel(0, priv + INTEL_PRIV_RESETS);
> +	writel(0, priv + INTEL_RESETS);
>  	timeout = jiffies + msecs_to_jiffies(10);
> -	while (!(readl(priv + INTEL_PRIV_RESETS) &
> -		 INTEL_PRIV_RESETS_RESET_DONE)) {
> +	while (!(readl(priv + INTEL_RESETS) &
> +		 INTEL_RESETS_RESET_DONE)) {
>  		if (time_after(jiffies, timeout))
>  			break;
>  		cpu_relax();
>  	}
> -	writel(INTEL_PRIV_RESETS_RESET, priv + INTEL_PRIV_RESETS);
> +	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
>
>  	return 0;
>  }
>
>  static struct mipi_i3c_hci_pci_info intel_info = {
> -	.init = mipi_i3c_hci_pci_intel_init,
> +	.init = intel_init,
>  };
>
>  static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 03/13] i3c: mipi-i3c-hci-pci: Rename some Intel-related identifiers

[Adrian Hunter]

On 12/11/2025 17:57, Frank Li wrote:
> On Wed, Nov 12, 2025 at 12:03:29PM +0200, Adrian Hunter wrote:
>> Rename some Intel-related identifiers to ensure Intel-related identifiers
>> begin "intel_" or INTEL_, and for brevity.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> index 617826628dc4..c74cc511a344 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> @@ -21,11 +21,11 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>>
>>  #define INTEL_PRIV_OFFSET		0x2b0
>>  #define INTEL_PRIV_SIZE			0x28
>> -#define INTEL_PRIV_RESETS		0x04
>> -#define INTEL_PRIV_RESETS_RESET		BIT(0)
>> -#define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
>> +#define INTEL_RESETS			0x04
>> +#define INTEL_RESETS_RESET		BIT(0)
>> +#define INTEL_RESETS_RESET_DONE		BIT(1)
> 
> These are already begin with INTEL.

For brevity.  They are long and "PRIV" adds nothing.

> 
>>
>> -static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
>> +static int intel_init(struct pci_dev *pci)
> 
> intel_i3c_init() ?
> 
> Frank
> 
>>  {
>>  	unsigned long timeout;
>>  	void __iomem *priv;
>> @@ -42,21 +42,21 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
>>  		return ret;
>>
>>  	/* Assert reset, wait for completion and release reset */
>> -	writel(0, priv + INTEL_PRIV_RESETS);
>> +	writel(0, priv + INTEL_RESETS);
>>  	timeout = jiffies + msecs_to_jiffies(10);
>> -	while (!(readl(priv + INTEL_PRIV_RESETS) &
>> -		 INTEL_PRIV_RESETS_RESET_DONE)) {
>> +	while (!(readl(priv + INTEL_RESETS) &
>> +		 INTEL_RESETS_RESET_DONE)) {
>>  		if (time_after(jiffies, timeout))
>>  			break;
>>  		cpu_relax();
>>  	}
>> -	writel(INTEL_PRIV_RESETS_RESET, priv + INTEL_PRIV_RESETS);
>> +	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
>>
>>  	return 0;
>>  }
>>
>>  static struct mipi_i3c_hci_pci_info intel_info = {
>> -	.init = mipi_i3c_hci_pci_intel_init,
>> +	.init = intel_init,
>>  };
>>
>>  static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>> --
>> 2.51.0
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 03/13] i3c: mipi-i3c-hci-pci: Rename some Intel-related identifiers

[Frank Li]

On Thu, Nov 13, 2025 at 03:01:02PM +0200, Adrian Hunter wrote:
> On 12/11/2025 17:57, Frank Li wrote:
> > On Wed, Nov 12, 2025 at 12:03:29PM +0200, Adrian Hunter wrote:
> >> Rename some Intel-related identifiers to ensure Intel-related identifiers
> >> begin "intel_" or INTEL_, and for brevity.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>  .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 18 +++++++++---------
> >>  1 file changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> index 617826628dc4..c74cc511a344 100644
> >> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> @@ -21,11 +21,11 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> >>
> >>  #define INTEL_PRIV_OFFSET		0x2b0
> >>  #define INTEL_PRIV_SIZE			0x28
> >> -#define INTEL_PRIV_RESETS		0x04
> >> -#define INTEL_PRIV_RESETS_RESET		BIT(0)
> >> -#define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
> >> +#define INTEL_RESETS			0x04
> >> +#define INTEL_RESETS_RESET		BIT(0)
> >> +#define INTEL_RESETS_RESET_DONE		BIT(1)
> >
> > These are already begin with INTEL.
>
> For brevity.  They are long and "PRIV" adds nothing.

Generally, needn't this type rename unless you need add more registers
defination later and want to keep consistent.

Frank

>
> >
> >>
> >> -static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
> >> +static int intel_init(struct pci_dev *pci)
> >
> > intel_i3c_init() ?
> >
> > Frank
> >
> >>  {
> >>  	unsigned long timeout;
> >>  	void __iomem *priv;
> >> @@ -42,21 +42,21 @@ static int mipi_i3c_hci_pci_intel_init(struct pci_dev *pci)
> >>  		return ret;
> >>
> >>  	/* Assert reset, wait for completion and release reset */
> >> -	writel(0, priv + INTEL_PRIV_RESETS);
> >> +	writel(0, priv + INTEL_RESETS);
> >>  	timeout = jiffies + msecs_to_jiffies(10);
> >> -	while (!(readl(priv + INTEL_PRIV_RESETS) &
> >> -		 INTEL_PRIV_RESETS_RESET_DONE)) {
> >> +	while (!(readl(priv + INTEL_RESETS) &
> >> +		 INTEL_RESETS_RESET_DONE)) {
> >>  		if (time_after(jiffies, timeout))
> >>  			break;
> >>  		cpu_relax();
> >>  	}
> >> -	writel(INTEL_PRIV_RESETS_RESET, priv + INTEL_PRIV_RESETS);
> >> +	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
> >>
> >>  	return 0;
> >>  }
> >>
> >>  static struct mipi_i3c_hci_pci_info intel_info = {
> >> -	.init = mipi_i3c_hci_pci_intel_init,
> >> +	.init = intel_init,
> >>  };
> >>
> >>  static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >> --
> >> 2.51.0
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 03/13] i3c: mipi-i3c-hci-pci: Rename some Intel-related identifiers

[Adrian Hunter]

On 13/11/2025 18:19, Frank Li wrote:
> On Thu, Nov 13, 2025 at 03:01:02PM +0200, Adrian Hunter wrote:
>> On 12/11/2025 17:57, Frank Li wrote:
>>> On Wed, Nov 12, 2025 at 12:03:29PM +0200, Adrian Hunter wrote:
>>>> Rename some Intel-related identifiers to ensure Intel-related identifiers
>>>> begin "intel_" or INTEL_, and for brevity.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>  .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 18 +++++++++---------
>>>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> index 617826628dc4..c74cc511a344 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> @@ -21,11 +21,11 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>>>>
>>>>  #define INTEL_PRIV_OFFSET		0x2b0
>>>>  #define INTEL_PRIV_SIZE			0x28
>>>> -#define INTEL_PRIV_RESETS		0x04
>>>> -#define INTEL_PRIV_RESETS_RESET		BIT(0)
>>>> -#define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
>>>> +#define INTEL_RESETS			0x04
>>>> +#define INTEL_RESETS_RESET		BIT(0)
>>>> +#define INTEL_RESETS_RESET_DONE		BIT(1)
>>>
>>> These are already begin with INTEL.
>>
>> For brevity.  They are long and "PRIV" adds nothing.
> 
> Generally, needn't this type rename unless you need add more registers
> defination later and want to keep consistent.

Patch 12 adds:

#define INTEL_ACTIVELTR			0x0c
#define INTEL_IDLELTR			0x10

#define INTEL_LTR_REQ			BIT(15)
#define INTEL_LTR_SCALE_MASK		GENMASK(11, 10)
#define INTEL_LTR_SCALE_1US		(2 << 10)
#define INTEL_LTR_SCALE_32US		(3 << 10)
#define INTEL_LTR_VALUE_MASK		GENMASK(9, 0)

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 03/13] i3c: mipi-i3c-hci-pci: Rename some Intel-related identifiers

[Frank Li]

On Thu, Nov 13, 2025 at 06:57:41PM +0200, Adrian Hunter wrote:
> On 13/11/2025 18:19, Frank Li wrote:
> > On Thu, Nov 13, 2025 at 03:01:02PM +0200, Adrian Hunter wrote:
> >> On 12/11/2025 17:57, Frank Li wrote:
> >>> On Wed, Nov 12, 2025 at 12:03:29PM +0200, Adrian Hunter wrote:
> >>>> Rename some Intel-related identifiers to ensure Intel-related identifiers
> >>>> begin "intel_" or INTEL_, and for brevity.
> >>>>
> >>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>>> ---
> >>>>  .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 18 +++++++++---------
> >>>>  1 file changed, 9 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> index 617826628dc4..c74cc511a344 100644
> >>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> @@ -21,11 +21,11 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> >>>>
> >>>>  #define INTEL_PRIV_OFFSET		0x2b0
> >>>>  #define INTEL_PRIV_SIZE			0x28
> >>>> -#define INTEL_PRIV_RESETS		0x04
> >>>> -#define INTEL_PRIV_RESETS_RESET		BIT(0)
> >>>> -#define INTEL_PRIV_RESETS_RESET_DONE	BIT(1)
> >>>> +#define INTEL_RESETS			0x04
> >>>> +#define INTEL_RESETS_RESET		BIT(0)
> >>>> +#define INTEL_RESETS_RESET_DONE		BIT(1)
> >>>
> >>> These are already begin with INTEL.
> >>
> >> For brevity.  They are long and "PRIV" adds nothing.
> >
> > Generally, needn't this type rename unless you need add more registers
> > defination later and want to keep consistent.
>
> Patch 12 adds:
>
> #define INTEL_ACTIVELTR			0x0c
> #define INTEL_IDLELTR			0x10
>
> #define INTEL_LTR_REQ			BIT(15)
> #define INTEL_LTR_SCALE_MASK		GENMASK(11, 10)
> #define INTEL_LTR_SCALE_1US		(2 << 10)
> #define INTEL_LTR_SCALE_32US		(3 << 10)
> #define INTEL_LTR_VALUE_MASK		GENMASK(9, 0)

You need mention these informaiton at commit message, such as prepare add
more register definiation and remove redunction "_PRIV_" to keep consistent

You can rephrase it. each commit message is independent.

Frank

>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 04/13] i3c: mipi-i3c-hci-pci: Use readl_poll_timeout()

[Adrian Hunter]

Use readl_poll_timeout() instead of open-coding the polling loop.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index c74cc511a344..c15e0556a691 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -8,6 +8,7 @@
  */
 #include <linux/acpi.h>
 #include <linux/idr.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -24,11 +25,12 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
 #define INTEL_RESETS			0x04
 #define INTEL_RESETS_RESET		BIT(0)
 #define INTEL_RESETS_RESET_DONE		BIT(1)
+#define INTEL_RESETS_TIMEOUT_US		10000
 
 static int intel_init(struct pci_dev *pci)
 {
-	unsigned long timeout;
 	void __iomem *priv;
+	u32 reg;
 	int ret;
 
 	priv = devm_ioremap(&pci->dev,
@@ -43,13 +45,9 @@ static int intel_init(struct pci_dev *pci)
 
 	/* Assert reset, wait for completion and release reset */
 	writel(0, priv + INTEL_RESETS);
-	timeout = jiffies + msecs_to_jiffies(10);
-	while (!(readl(priv + INTEL_RESETS) &
-		 INTEL_RESETS_RESET_DONE)) {
-		if (time_after(jiffies, timeout))
-			break;
-		cpu_relax();
-	}
+	readl_poll_timeout(priv + INTEL_RESETS, reg,
+			   reg & INTEL_RESETS_RESET_DONE, 0,
+			   INTEL_RESETS_TIMEOUT_US);
 	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
 
 	return 0;
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 04/13] i3c: mipi-i3c-hci-pci: Use readl_poll_timeout()

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:30PM +0200, Adrian Hunter wrote:
> Use readl_poll_timeout() instead of open-coding the polling loop.

nice
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index c74cc511a344..c15e0556a691 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -8,6 +8,7 @@
>   */
>  #include <linux/acpi.h>
>  #include <linux/idr.h>
> +#include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> @@ -24,11 +25,12 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>  #define INTEL_RESETS			0x04
>  #define INTEL_RESETS_RESET		BIT(0)
>  #define INTEL_RESETS_RESET_DONE		BIT(1)
> +#define INTEL_RESETS_TIMEOUT_US		10000

10 * USEC_PER_MSEC

Frank
>
>  static int intel_init(struct pci_dev *pci)
>  {
> -	unsigned long timeout;
>  	void __iomem *priv;
> +	u32 reg;
>  	int ret;
>
>  	priv = devm_ioremap(&pci->dev,
> @@ -43,13 +45,9 @@ static int intel_init(struct pci_dev *pci)
>
>  	/* Assert reset, wait for completion and release reset */
>  	writel(0, priv + INTEL_RESETS);
> -	timeout = jiffies + msecs_to_jiffies(10);
> -	while (!(readl(priv + INTEL_RESETS) &
> -		 INTEL_RESETS_RESET_DONE)) {
> -		if (time_after(jiffies, timeout))
> -			break;
> -		cpu_relax();
> -	}
> +	readl_poll_timeout(priv + INTEL_RESETS, reg,
> +			   reg & INTEL_RESETS_RESET_DONE, 0,
> +			   INTEL_RESETS_TIMEOUT_US);
>  	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
>
>  	return 0;
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 04/13] i3c: mipi-i3c-hci-pci: Use readl_poll_timeout()

[Alexandre Belloni]

On 12/11/2025 10:59:57-0500, Frank Li wrote:
> On Wed, Nov 12, 2025 at 12:03:30PM +0200, Adrian Hunter wrote:
> > Use readl_poll_timeout() instead of open-coding the polling loop.
> 
> nice
> >
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > ---
> >  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > index c74cc511a344..c15e0556a691 100644
> > --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > @@ -8,6 +8,7 @@
> >   */
> >  #include <linux/acpi.h>
> >  #include <linux/idr.h>
> > +#include <linux/iopoll.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/pci.h>
> > @@ -24,11 +25,12 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> >  #define INTEL_RESETS			0x04
> >  #define INTEL_RESETS_RESET		BIT(0)
> >  #define INTEL_RESETS_RESET_DONE		BIT(1)
> > +#define INTEL_RESETS_TIMEOUT_US		10000
> 
> 10 * USEC_PER_MSEC

This is super hard nitpicking, I guess everyone knows 10000µs is 10ms

> 
> Frank
> >
> >  static int intel_init(struct pci_dev *pci)
> >  {
> > -	unsigned long timeout;
> >  	void __iomem *priv;
> > +	u32 reg;
> >  	int ret;
> >
> >  	priv = devm_ioremap(&pci->dev,
> > @@ -43,13 +45,9 @@ static int intel_init(struct pci_dev *pci)
> >
> >  	/* Assert reset, wait for completion and release reset */
> >  	writel(0, priv + INTEL_RESETS);
> > -	timeout = jiffies + msecs_to_jiffies(10);
> > -	while (!(readl(priv + INTEL_RESETS) &
> > -		 INTEL_RESETS_RESET_DONE)) {
> > -		if (time_after(jiffies, timeout))
> > -			break;
> > -		cpu_relax();
> > -	}
> > +	readl_poll_timeout(priv + INTEL_RESETS, reg,
> > +			   reg & INTEL_RESETS_RESET_DONE, 0,
> > +			   INTEL_RESETS_TIMEOUT_US);
> >  	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
> >
> >  	return 0;
> > --
> > 2.51.0
> >
> >
> > --
> > linux-i3c mailing list
> > linux-i3c@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 04/13] i3c: mipi-i3c-hci-pci: Use readl_poll_timeout()

[Frank Li]

On Fri, Nov 14, 2025 at 05:02:05PM +0100, Alexandre Belloni wrote:
> On 12/11/2025 10:59:57-0500, Frank Li wrote:
> > On Wed, Nov 12, 2025 at 12:03:30PM +0200, Adrian Hunter wrote:
> > > Use readl_poll_timeout() instead of open-coding the polling loop.
> >
> > nice
> > >
> > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > > ---
> > >  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 14 ++++++--------
> > >  1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > > index c74cc511a344..c15e0556a691 100644
> > > --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > > +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > > @@ -8,6 +8,7 @@
> > >   */
> > >  #include <linux/acpi.h>
> > >  #include <linux/idr.h>
> > > +#include <linux/iopoll.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/module.h>
> > >  #include <linux/pci.h>
> > > @@ -24,11 +25,12 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> > >  #define INTEL_RESETS			0x04
> > >  #define INTEL_RESETS_RESET		BIT(0)
> > >  #define INTEL_RESETS_RESET_DONE		BIT(1)
> > > +#define INTEL_RESETS_TIMEOUT_US		10000
> >
> > 10 * USEC_PER_MSEC
>
> This is super hard nitpicking, I guess everyone knows 10000µs is 10ms

It is from
https://lore.kernel.org/all/aPnrc-Z5WVJorGr7@smile.fi.intel.com/

Generally, I only provide such nit at early version.

I think benefit is avoid miss count 0. Maybe C23, 10'000, especially when
number is big.

Frank
>
> >
> > Frank
> > >
> > >  static int intel_init(struct pci_dev *pci)
> > >  {
> > > -	unsigned long timeout;
> > >  	void __iomem *priv;
> > > +	u32 reg;
> > >  	int ret;
> > >
> > >  	priv = devm_ioremap(&pci->dev,
> > > @@ -43,13 +45,9 @@ static int intel_init(struct pci_dev *pci)
> > >
> > >  	/* Assert reset, wait for completion and release reset */
> > >  	writel(0, priv + INTEL_RESETS);
> > > -	timeout = jiffies + msecs_to_jiffies(10);
> > > -	while (!(readl(priv + INTEL_RESETS) &
> > > -		 INTEL_RESETS_RESET_DONE)) {
> > > -		if (time_after(jiffies, timeout))
> > > -			break;
> > > -		cpu_relax();
> > > -	}
> > > +	readl_poll_timeout(priv + INTEL_RESETS, reg,
> > > +			   reg & INTEL_RESETS_RESET_DONE, 0,
> > > +			   INTEL_RESETS_TIMEOUT_US);
> > >  	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
> > >
> > >  	return 0;
> > > --
> > > 2.51.0
> > >
> > >
> > > --
> > > linux-i3c mailing list
> > > linux-i3c@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-i3c
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 04/13] i3c: mipi-i3c-hci-pci: Use readl_poll_timeout()

[Alexandre Belloni]

On 14/11/2025 11:43:36-0500, Frank Li wrote:
> On Fri, Nov 14, 2025 at 05:02:05PM +0100, Alexandre Belloni wrote:
> > On 12/11/2025 10:59:57-0500, Frank Li wrote:
> > > On Wed, Nov 12, 2025 at 12:03:30PM +0200, Adrian Hunter wrote:
> > > > Use readl_poll_timeout() instead of open-coding the polling loop.
> > >
> > > nice
> > > >
> > > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > > > ---
> > > >  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 14 ++++++--------
> > > >  1 file changed, 6 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > > > index c74cc511a344..c15e0556a691 100644
> > > > --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > > > +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> > > > @@ -8,6 +8,7 @@
> > > >   */
> > > >  #include <linux/acpi.h>
> > > >  #include <linux/idr.h>
> > > > +#include <linux/iopoll.h>
> > > >  #include <linux/kernel.h>
> > > >  #include <linux/module.h>
> > > >  #include <linux/pci.h>
> > > > @@ -24,11 +25,12 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> > > >  #define INTEL_RESETS			0x04
> > > >  #define INTEL_RESETS_RESET		BIT(0)
> > > >  #define INTEL_RESETS_RESET_DONE		BIT(1)
> > > > +#define INTEL_RESETS_TIMEOUT_US		10000
> > >
> > > 10 * USEC_PER_MSEC
> >
> > This is super hard nitpicking, I guess everyone knows 10000µs is 10ms
> 
> It is from
> https://lore.kernel.org/all/aPnrc-Z5WVJorGr7@smile.fi.intel.com/
> 

Well, Andy is super nitpicky. I guess 10000 has US just before so there
won't be any confusion on the actual unit

In your patch, I wouldn't change it for MMC5633_WAIT_SET_RESET_US but I
agree the call to regmap_read_poll_timeout needs to be clearer.

> Generally, I only provide such nit at early version.
> 
> I think benefit is avoid miss count 0. Maybe C23, 10'000, especially when
> number is big.
> 
> Frank
> >
> > >
> > > Frank
> > > >
> > > >  static int intel_init(struct pci_dev *pci)
> > > >  {
> > > > -	unsigned long timeout;
> > > >  	void __iomem *priv;
> > > > +	u32 reg;
> > > >  	int ret;
> > > >
> > > >  	priv = devm_ioremap(&pci->dev,
> > > > @@ -43,13 +45,9 @@ static int intel_init(struct pci_dev *pci)
> > > >
> > > >  	/* Assert reset, wait for completion and release reset */
> > > >  	writel(0, priv + INTEL_RESETS);
> > > > -	timeout = jiffies + msecs_to_jiffies(10);
> > > > -	while (!(readl(priv + INTEL_RESETS) &
> > > > -		 INTEL_RESETS_RESET_DONE)) {
> > > > -		if (time_after(jiffies, timeout))
> > > > -			break;
> > > > -		cpu_relax();
> > > > -	}
> > > > +	readl_poll_timeout(priv + INTEL_RESETS, reg,
> > > > +			   reg & INTEL_RESETS_RESET_DONE, 0,
> > > > +			   INTEL_RESETS_TIMEOUT_US);
> > > >  	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
> > > >
> > > >  	return 0;
> > > > --
> > > > 2.51.0
> > > >
> > > >
> > > > --
> > > > linux-i3c mailing list
> > > > linux-i3c@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-i3c
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> >
> > --
> > linux-i3c mailing list
> > linux-i3c@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 05/13] i3c: mipi-i3c-hci-pci: Constify driver data

[Adrian Hunter]

Add const qualifier to driver data because it is constant.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index c15e0556a691..59c592446b5c 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -53,14 +53,14 @@ static int intel_init(struct pci_dev *pci)
 	return 0;
 }
 
-static struct mipi_i3c_hci_pci_info intel_info = {
+static const struct mipi_i3c_hci_pci_info intel_info = {
 	.init = intel_init,
 };
 
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 				  const struct pci_device_id *id)
 {
-	struct mipi_i3c_hci_pci_info *info;
+	const struct mipi_i3c_hci_pci_info *info;
 	struct platform_device *pdev;
 	struct resource res[2];
 	int dev_id, ret;
@@ -96,7 +96,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	if (ret)
 		goto err;
 
-	info = (struct mipi_i3c_hci_pci_info *)id->driver_data;
+	info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
 	if (info && info->init) {
 		ret = info->init(pci);
 		if (ret)
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 05/13] i3c: mipi-i3c-hci-pci: Constify driver data

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:31PM +0200, Adrian Hunter wrote:
> Add const qualifier to driver data because it is constant.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index c15e0556a691..59c592446b5c 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -53,14 +53,14 @@ static int intel_init(struct pci_dev *pci)
>  	return 0;
>  }
>
> -static struct mipi_i3c_hci_pci_info intel_info = {
> +static const struct mipi_i3c_hci_pci_info intel_info = {
>  	.init = intel_init,
>  };
>
>  static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  				  const struct pci_device_id *id)
>  {
> -	struct mipi_i3c_hci_pci_info *info;
> +	const struct mipi_i3c_hci_pci_info *info;
>  	struct platform_device *pdev;
>  	struct resource res[2];
>  	int dev_id, ret;
> @@ -96,7 +96,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  	if (ret)
>  		goto err;
>
> -	info = (struct mipi_i3c_hci_pci_info *)id->driver_data;
> +	info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
>  	if (info && info->init) {
>  		ret = info->init(pci);
>  		if (ret)
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 06/13] i3c: mipi-i3c-hci-pci: Factor out private registers ioremapping

[Adrian Hunter]

For neatness, factor out private registers ioremapping.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 59c592446b5c..003ae61e9f6c 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -27,15 +27,19 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
 #define INTEL_RESETS_RESET_DONE		BIT(1)
 #define INTEL_RESETS_TIMEOUT_US		10000
 
+static void __iomem *intel_priv(struct pci_dev *pci)
+{
+	resource_size_t base = pci_resource_start(pci, 0);
+
+	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
+}
+
 static int intel_init(struct pci_dev *pci)
 {
-	void __iomem *priv;
+	void __iomem *priv = intel_priv(pci);
 	u32 reg;
 	int ret;
 
-	priv = devm_ioremap(&pci->dev,
-			    pci_resource_start(pci, 0) + INTEL_PRIV_OFFSET,
-			    INTEL_PRIV_SIZE);
 	if (!priv)
 		return -ENOMEM;
 
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 06/13] i3c: mipi-i3c-hci-pci: Factor out private registers ioremapping

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:32PM +0200, Adrian Hunter wrote:
> For neatness, factor out private registers ioremapping.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

feel like not beanfit.

Frank

>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 59c592446b5c..003ae61e9f6c 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -27,15 +27,19 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>  #define INTEL_RESETS_RESET_DONE		BIT(1)
>  #define INTEL_RESETS_TIMEOUT_US		10000
>
> +static void __iomem *intel_priv(struct pci_dev *pci)
> +{
> +	resource_size_t base = pci_resource_start(pci, 0);
> +
> +	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
> +}
> +
>  static int intel_init(struct pci_dev *pci)
>  {
> -	void __iomem *priv;
> +	void __iomem *priv = intel_priv(pci);
>  	u32 reg;
>  	int ret;
>
> -	priv = devm_ioremap(&pci->dev,
> -			    pci_resource_start(pci, 0) + INTEL_PRIV_OFFSET,
> -			    INTEL_PRIV_SIZE);
>  	if (!priv)
>  		return -ENOMEM;
>
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 06/13] i3c: mipi-i3c-hci-pci: Factor out private registers ioremapping

[Adrian Hunter]

On 12/11/2025 18:09, Frank Li wrote:
> On Wed, Nov 12, 2025 at 12:03:32PM +0200, Adrian Hunter wrote:
>> For neatness, factor out private registers ioremapping.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
> 
> feel like not beanfit.

The benefit is that it is neater and more readable.

> 
> Frank
> 
>>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> index 59c592446b5c..003ae61e9f6c 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> @@ -27,15 +27,19 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>>  #define INTEL_RESETS_RESET_DONE		BIT(1)
>>  #define INTEL_RESETS_TIMEOUT_US		10000
>>
>> +static void __iomem *intel_priv(struct pci_dev *pci)
>> +{
>> +	resource_size_t base = pci_resource_start(pci, 0);
>> +
>> +	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
>> +}
>> +
>>  static int intel_init(struct pci_dev *pci)
>>  {
>> -	void __iomem *priv;
>> +	void __iomem *priv = intel_priv(pci);
>>  	u32 reg;
>>  	int ret;
>>
>> -	priv = devm_ioremap(&pci->dev,
>> -			    pci_resource_start(pci, 0) + INTEL_PRIV_OFFSET,
>> -			    INTEL_PRIV_SIZE);
>>  	if (!priv)
>>  		return -ENOMEM;
>>
>> --
>> 2.51.0
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 07/13] i3c: mipi-i3c-hci-pci: Factor out intel_reset()

[Adrian Hunter]

For neatness, factor out intel_reset().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 26 ++++++++++++-------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 003ae61e9f6c..b3b6b6f43af2 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -34,18 +34,9 @@ static void __iomem *intel_priv(struct pci_dev *pci)
 	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
 }
 
-static int intel_init(struct pci_dev *pci)
+static void intel_reset(void __iomem *priv)
 {
-	void __iomem *priv = intel_priv(pci);
 	u32 reg;
-	int ret;
-
-	if (!priv)
-		return -ENOMEM;
-
-	ret = dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(64));
-	if (ret)
-		return ret;
 
 	/* Assert reset, wait for completion and release reset */
 	writel(0, priv + INTEL_RESETS);
@@ -53,6 +44,21 @@ static int intel_init(struct pci_dev *pci)
 			   reg & INTEL_RESETS_RESET_DONE, 0,
 			   INTEL_RESETS_TIMEOUT_US);
 	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
+}
+
+static int intel_init(struct pci_dev *pci)
+{
+	void __iomem *priv = intel_priv(pci);
+	int ret;
+
+	if (!priv)
+		return -ENOMEM;
+
+	ret = dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(64));
+	if (ret)
+		return ret;
+
+	intel_reset(priv);
 
 	return 0;
 }
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 07/13] i3c: mipi-i3c-hci-pci: Factor out intel_reset()

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:33PM +0200, Adrian Hunter wrote:
> For neatness, factor out intel_reset().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 26 ++++++++++++-------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 003ae61e9f6c..b3b6b6f43af2 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -34,18 +34,9 @@ static void __iomem *intel_priv(struct pci_dev *pci)
>  	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
>  }
>
> -static int intel_init(struct pci_dev *pci)
> +static void intel_reset(void __iomem *priv)

can move intel_reset() little far from intel_init() to make patch look
nice.

Frank

>  {
> -	void __iomem *priv = intel_priv(pci);
>  	u32 reg;
> -	int ret;
> -
> -	if (!priv)
> -		return -ENOMEM;
> -
> -	ret = dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(64));
> -	if (ret)
> -		return ret;
>
>  	/* Assert reset, wait for completion and release reset */
>  	writel(0, priv + INTEL_RESETS);
> @@ -53,6 +44,21 @@ static int intel_init(struct pci_dev *pci)
>  			   reg & INTEL_RESETS_RESET_DONE, 0,
>  			   INTEL_RESETS_TIMEOUT_US);
>  	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
> +}
> +
> +static int intel_init(struct pci_dev *pci)
> +{
> +	void __iomem *priv = intel_priv(pci);
> +	int ret;
> +
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(64));
> +	if (ret)
> +		return ret;
> +
> +	intel_reset(priv);
>
>  	return 0;
>  }
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 08/13] i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information

[Adrian Hunter]

Allocate a structure for mipi_i3c_hci_pci device information, in
preparation for additional changes that need to store mipi_i3c_hci_pci
device-specific information.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 29 ++++++++++++-------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index b3b6b6f43af2..9fa89af7479f 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -14,6 +14,10 @@
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 
+struct mipi_i3c_hci_pci {
+	struct platform_device *pdev;
+};
+
 struct mipi_i3c_hci_pci_info {
 	int (*init)(struct pci_dev *pci);
 };
@@ -71,10 +75,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 				  const struct pci_device_id *id)
 {
 	const struct mipi_i3c_hci_pci_info *info;
-	struct platform_device *pdev;
+	struct mipi_i3c_hci_pci *hci;
 	struct resource res[2];
 	int dev_id, ret;
 
+	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
+	if (!hci)
+		return -ENOMEM;
+
 	ret = pcim_enable_device(pci);
 	if (ret)
 		return ret;
@@ -95,14 +103,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	if (dev_id < 0)
 		return dev_id;
 
-	pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
-	if (!pdev)
+	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
+	if (!hci->pdev)
 		return -ENOMEM;
 
-	pdev->dev.parent = &pci->dev;
-	device_set_node(&pdev->dev, dev_fwnode(&pci->dev));
+	hci->pdev->dev.parent = &pci->dev;
+	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
 
-	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
+	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
 	if (ret)
 		goto err;
 
@@ -113,23 +121,24 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 			goto err;
 	}
 
-	ret = platform_device_add(pdev);
+	ret = platform_device_add(hci->pdev);
 	if (ret)
 		goto err;
 
-	pci_set_drvdata(pci, pdev);
+	pci_set_drvdata(pci, hci);
 
 	return 0;
 
 err:
-	platform_device_put(pdev);
+	platform_device_put(hci->pdev);
 	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
 	return ret;
 }
 
 static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 {
-	struct platform_device *pdev = pci_get_drvdata(pci);
+	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
+	struct platform_device *pdev = hci->pdev;
 	int dev_id = pdev->id;
 
 	platform_device_unregister(pdev);
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 08/13] i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information

[Frank Li]

On Wed, Nov 12, 2025 at 12:03:34PM +0200, Adrian Hunter wrote:
> Allocate a structure for mipi_i3c_hci_pci device information, in
> preparation for additional changes that need to store mipi_i3c_hci_pci
> device-specific information.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 29 ++++++++++++-------
>  1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index b3b6b6f43af2..9fa89af7479f 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -14,6 +14,10 @@
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
>
> +struct mipi_i3c_hci_pci {
> +	struct platform_device *pdev;
> +};
> +

Is it simpler by using platform_device_register_data(), pass down platform
related information by void *data?

Frank
>  struct mipi_i3c_hci_pci_info {
>  	int (*init)(struct pci_dev *pci);
>  };
> @@ -71,10 +75,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  				  const struct pci_device_id *id)
>  {
>  	const struct mipi_i3c_hci_pci_info *info;
> -	struct platform_device *pdev;
> +	struct mipi_i3c_hci_pci *hci;
>  	struct resource res[2];
>  	int dev_id, ret;
>
> +	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
> +	if (!hci)
> +		return -ENOMEM;
> +
>  	ret = pcim_enable_device(pci);
>  	if (ret)
>  		return ret;
> @@ -95,14 +103,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  	if (dev_id < 0)
>  		return dev_id;
>
> -	pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> -	if (!pdev)
> +	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> +	if (!hci->pdev)
>  		return -ENOMEM;
>
> -	pdev->dev.parent = &pci->dev;
> -	device_set_node(&pdev->dev, dev_fwnode(&pci->dev));
> +	hci->pdev->dev.parent = &pci->dev;
> +	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
>
> -	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
> +	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
>  	if (ret)
>  		goto err;
>
> @@ -113,23 +121,24 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  			goto err;
>  	}
>
> -	ret = platform_device_add(pdev);
> +	ret = platform_device_add(hci->pdev);
>  	if (ret)
>  		goto err;
>
> -	pci_set_drvdata(pci, pdev);
> +	pci_set_drvdata(pci, hci);
>
>  	return 0;
>
>  err:
> -	platform_device_put(pdev);
> +	platform_device_put(hci->pdev);
>  	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
>  	return ret;
>  }
>
>  static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>  {
> -	struct platform_device *pdev = pci_get_drvdata(pci);
> +	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
> +	struct platform_device *pdev = hci->pdev;
>  	int dev_id = pdev->id;
>
>  	platform_device_unregister(pdev);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 08/13] i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information

[Adrian Hunter]

On 12/11/2025 18:41, Frank Li wrote:
> On Wed, Nov 12, 2025 at 12:03:34PM +0200, Adrian Hunter wrote:
>> Allocate a structure for mipi_i3c_hci_pci device information, in
>> preparation for additional changes that need to store mipi_i3c_hci_pci
>> device-specific information.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 29 ++++++++++++-------
>>  1 file changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> index b3b6b6f43af2..9fa89af7479f 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> @@ -14,6 +14,10 @@
>>  #include <linux/pci.h>
>>  #include <linux/platform_device.h>
>>
>> +struct mipi_i3c_hci_pci {
>> +	struct platform_device *pdev;
>> +};
>> +
> 
> Is it simpler by using platform_device_register_data(), pass down platform
> related information by void *data?

No, this is just a normal allocated structure for the PCI driver
(mipi_i3c_hci_pci) to put information related to the PCI device.
It is later used to store the current Latency Tolerance Reporting (LTR)
register values.

So it is not the same thing as platform_data.

> 
> Frank
>>  struct mipi_i3c_hci_pci_info {
>>  	int (*init)(struct pci_dev *pci);
>>  };
>> @@ -71,10 +75,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>  				  const struct pci_device_id *id)
>>  {
>>  	const struct mipi_i3c_hci_pci_info *info;
>> -	struct platform_device *pdev;
>> +	struct mipi_i3c_hci_pci *hci;
>>  	struct resource res[2];
>>  	int dev_id, ret;
>>
>> +	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
>> +	if (!hci)
>> +		return -ENOMEM;
>> +
>>  	ret = pcim_enable_device(pci);
>>  	if (ret)
>>  		return ret;
>> @@ -95,14 +103,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>  	if (dev_id < 0)
>>  		return dev_id;
>>
>> -	pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
>> -	if (!pdev)
>> +	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
>> +	if (!hci->pdev)
>>  		return -ENOMEM;
>>
>> -	pdev->dev.parent = &pci->dev;
>> -	device_set_node(&pdev->dev, dev_fwnode(&pci->dev));
>> +	hci->pdev->dev.parent = &pci->dev;
>> +	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
>>
>> -	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
>> +	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
>>  	if (ret)
>>  		goto err;
>>
>> @@ -113,23 +121,24 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>  			goto err;
>>  	}
>>
>> -	ret = platform_device_add(pdev);
>> +	ret = platform_device_add(hci->pdev);
>>  	if (ret)
>>  		goto err;
>>
>> -	pci_set_drvdata(pci, pdev);
>> +	pci_set_drvdata(pci, hci);
>>
>>  	return 0;
>>
>>  err:
>> -	platform_device_put(pdev);
>> +	platform_device_put(hci->pdev);
>>  	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
>>  	return ret;
>>  }
>>
>>  static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>>  {
>> -	struct platform_device *pdev = pci_get_drvdata(pci);
>> +	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
>> +	struct platform_device *pdev = hci->pdev;
>>  	int dev_id = pdev->id;
>>
>>  	platform_device_unregister(pdev);
>> --
>> 2.51.0
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 08/13] i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information

[Frank Li]

On Wed, Nov 12, 2025 at 09:25:14PM +0200, Adrian Hunter wrote:
> On 12/11/2025 18:41, Frank Li wrote:
> > On Wed, Nov 12, 2025 at 12:03:34PM +0200, Adrian Hunter wrote:
> >> Allocate a structure for mipi_i3c_hci_pci device information, in
> >> preparation for additional changes that need to store mipi_i3c_hci_pci
> >> device-specific information.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 29 ++++++++++++-------
> >>  1 file changed, 19 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> index b3b6b6f43af2..9fa89af7479f 100644
> >> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> @@ -14,6 +14,10 @@
> >>  #include <linux/pci.h>
> >>  #include <linux/platform_device.h>
> >>
> >> +struct mipi_i3c_hci_pci {
> >> +	struct platform_device *pdev;
> >> +};
> >> +
> >
> > Is it simpler by using platform_device_register_data(), pass down platform
> > related information by void *data?
>
> No, this is just a normal allocated structure for the PCI driver
> (mipi_i3c_hci_pci) to put information related to the PCI device.
> It is later used to store the current Latency Tolerance Reporting (LTR)
> register values.

Is it static value, or does it change after boot in other words?

>
> So it is not the same thing as platform_data.

Base on your code to create platform data is duplicate what's did by
platform_device_register_data().

The pci device create platform devices,  platform driver should not use
pci's information directly. platform driver should be reused if it probe
from acpi or dt-tree.

I think it'd better pass down platform data information, which can include
callback to fetch needed information for difference's parent devices.

Frank

>
> >
> > Frank
> >>  struct mipi_i3c_hci_pci_info {
> >>  	int (*init)(struct pci_dev *pci);
> >>  };
> >> @@ -71,10 +75,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>  				  const struct pci_device_id *id)
> >>  {
> >>  	const struct mipi_i3c_hci_pci_info *info;
> >> -	struct platform_device *pdev;
> >> +	struct mipi_i3c_hci_pci *hci;
> >>  	struct resource res[2];
> >>  	int dev_id, ret;
> >>
> >> +	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
> >> +	if (!hci)
> >> +		return -ENOMEM;
> >> +
> >>  	ret = pcim_enable_device(pci);
> >>  	if (ret)
> >>  		return ret;
> >> @@ -95,14 +103,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>  	if (dev_id < 0)
> >>  		return dev_id;
> >>
> >> -	pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> >> -	if (!pdev)
> >> +	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> >> +	if (!hci->pdev)
> >>  		return -ENOMEM;
> >>
> >> -	pdev->dev.parent = &pci->dev;
> >> -	device_set_node(&pdev->dev, dev_fwnode(&pci->dev));
> >> +	hci->pdev->dev.parent = &pci->dev;
> >> +	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
> >>
> >> -	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
> >> +	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
> >>  	if (ret)
> >>  		goto err;
> >>
> >> @@ -113,23 +121,24 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>  			goto err;
> >>  	}
> >>
> >> -	ret = platform_device_add(pdev);
> >> +	ret = platform_device_add(hci->pdev);
> >>  	if (ret)
> >>  		goto err;
> >>
> >> -	pci_set_drvdata(pci, pdev);
> >> +	pci_set_drvdata(pci, hci);
> >>
> >>  	return 0;
> >>
> >>  err:
> >> -	platform_device_put(pdev);
> >> +	platform_device_put(hci->pdev);
> >>  	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
> >>  	return ret;
> >>  }
> >>
> >>  static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
> >>  {
> >> -	struct platform_device *pdev = pci_get_drvdata(pci);
> >> +	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
> >> +	struct platform_device *pdev = hci->pdev;
> >>  	int dev_id = pdev->id;
> >>
> >>  	platform_device_unregister(pdev);
> >> --
> >> 2.51.0
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 08/13] i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information

[Adrian Hunter]

On 12/11/2025 22:17, Frank Li wrote:
> On Wed, Nov 12, 2025 at 09:25:14PM +0200, Adrian Hunter wrote:
>> On 12/11/2025 18:41, Frank Li wrote:
>>> On Wed, Nov 12, 2025 at 12:03:34PM +0200, Adrian Hunter wrote:
>>>> Allocate a structure for mipi_i3c_hci_pci device information, in
>>>> preparation for additional changes that need to store mipi_i3c_hci_pci
>>>> device-specific information.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 29 ++++++++++++-------
>>>>  1 file changed, 19 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> index b3b6b6f43af2..9fa89af7479f 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> @@ -14,6 +14,10 @@
>>>>  #include <linux/pci.h>
>>>>  #include <linux/platform_device.h>
>>>>
>>>> +struct mipi_i3c_hci_pci {
>>>> +	struct platform_device *pdev;
>>>> +};
>>>> +
>>>
>>> Is it simpler by using platform_device_register_data(), pass down platform
>>> related information by void *data?
>>
>> No, this is just a normal allocated structure for the PCI driver
>> (mipi_i3c_hci_pci) to put information related to the PCI device.
>> It is later used to store the current Latency Tolerance Reporting (LTR)
>> register values.
> 
> Is it static value, or does it change after boot in other words?

Dynamic

> 
>>
>> So it is not the same thing as platform_data.
> 
> Base on your code to create platform data is duplicate what's did by
> platform_device_register_data().

It is not platform data

> 
> The pci device create platform devices,  platform driver should not use
> pci's information directly. platform driver should be reused if it probe
> from acpi or dt-tree.
> 
> I think it'd better pass down platform data information, which can include
> callback to fetch needed information for difference's parent devices.

PCI Latency Tolerance Reporting (LTR) has nothing to do with I3C
or the platform device, so it does not belong in the platform driver.

> 
> Frank
> 
>>
>>>
>>> Frank
>>>>  struct mipi_i3c_hci_pci_info {
>>>>  	int (*init)(struct pci_dev *pci);
>>>>  };
>>>> @@ -71,10 +75,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>>>  				  const struct pci_device_id *id)
>>>>  {
>>>>  	const struct mipi_i3c_hci_pci_info *info;
>>>> -	struct platform_device *pdev;
>>>> +	struct mipi_i3c_hci_pci *hci;
>>>>  	struct resource res[2];
>>>>  	int dev_id, ret;
>>>>
>>>> +	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
>>>> +	if (!hci)
>>>> +		return -ENOMEM;
>>>> +
>>>>  	ret = pcim_enable_device(pci);
>>>>  	if (ret)
>>>>  		return ret;
>>>> @@ -95,14 +103,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>>>  	if (dev_id < 0)
>>>>  		return dev_id;
>>>>
>>>> -	pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
>>>> -	if (!pdev)
>>>> +	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
>>>> +	if (!hci->pdev)
>>>>  		return -ENOMEM;
>>>>
>>>> -	pdev->dev.parent = &pci->dev;
>>>> -	device_set_node(&pdev->dev, dev_fwnode(&pci->dev));
>>>> +	hci->pdev->dev.parent = &pci->dev;
>>>> +	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
>>>>
>>>> -	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
>>>> +	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
>>>>  	if (ret)
>>>>  		goto err;
>>>>
>>>> @@ -113,23 +121,24 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>>>  			goto err;
>>>>  	}
>>>>
>>>> -	ret = platform_device_add(pdev);
>>>> +	ret = platform_device_add(hci->pdev);
>>>>  	if (ret)
>>>>  		goto err;
>>>>
>>>> -	pci_set_drvdata(pci, pdev);
>>>> +	pci_set_drvdata(pci, hci);
>>>>
>>>>  	return 0;
>>>>
>>>>  err:
>>>> -	platform_device_put(pdev);
>>>> +	platform_device_put(hci->pdev);
>>>>  	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
>>>>  	return ret;
>>>>  }
>>>>
>>>>  static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>>>>  {
>>>> -	struct platform_device *pdev = pci_get_drvdata(pci);
>>>> +	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
>>>> +	struct platform_device *pdev = hci->pdev;
>>>>  	int dev_id = pdev->id;
>>>>
>>>>  	platform_device_unregister(pdev);
>>>> --
>>>> 2.51.0
>>>>
>>>>
>>>> --
>>>> linux-i3c mailing list
>>>> linux-i3c@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 08/13] i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information

[Frank Li]

On Thu, Nov 13, 2025 at 03:15:34PM +0200, Adrian Hunter wrote:
> On 12/11/2025 22:17, Frank Li wrote:
> > On Wed, Nov 12, 2025 at 09:25:14PM +0200, Adrian Hunter wrote:
> >> On 12/11/2025 18:41, Frank Li wrote:
> >>> On Wed, Nov 12, 2025 at 12:03:34PM +0200, Adrian Hunter wrote:
> >>>> Allocate a structure for mipi_i3c_hci_pci device information, in
> >>>> preparation for additional changes that need to store mipi_i3c_hci_pci
> >>>> device-specific information.
> >>>>
> >>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>>> ---
> >>>>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 29 ++++++++++++-------
> >>>>  1 file changed, 19 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> index b3b6b6f43af2..9fa89af7479f 100644
> >>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> @@ -14,6 +14,10 @@
> >>>>  #include <linux/pci.h>
> >>>>  #include <linux/platform_device.h>
> >>>>
> >>>> +struct mipi_i3c_hci_pci {
> >>>> +	struct platform_device *pdev;
> >>>> +};
> >>>> +
> >>>
> >>> Is it simpler by using platform_device_register_data(), pass down platform
> >>> related information by void *data?
> >>
> >> No, this is just a normal allocated structure for the PCI driver
> >> (mipi_i3c_hci_pci) to put information related to the PCI device.
> >> It is later used to store the current Latency Tolerance Reporting (LTR)
> >> register values.
> >
> > Is it static value, or does it change after boot in other words?
>
> Dynamic
>
> >
> >>
> >> So it is not the same thing as platform_data.
> >
> > Base on your code to create platform data is duplicate what's did by
> > platform_device_register_data().
>
> It is not platform data

I understand 'mipi_i3c_hci_pci' now.
I think you still use platform_device_register_data() to simple your code.

But need seperate patch to do it.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> >
> > The pci device create platform devices,  platform driver should not use
> > pci's information directly. platform driver should be reused if it probe
> > from acpi or dt-tree.
> >
> > I think it'd better pass down platform data information, which can include
> > callback to fetch needed information for difference's parent devices.
>
> PCI Latency Tolerance Reporting (LTR) has nothing to do with I3C
> or the platform device, so it does not belong in the platform driver.
>
> >
> > Frank
> >
> >>
> >>>
> >>> Frank
> >>>>  struct mipi_i3c_hci_pci_info {
> >>>>  	int (*init)(struct pci_dev *pci);
> >>>>  };
> >>>> @@ -71,10 +75,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>  				  const struct pci_device_id *id)
> >>>>  {
> >>>>  	const struct mipi_i3c_hci_pci_info *info;
> >>>> -	struct platform_device *pdev;
> >>>> +	struct mipi_i3c_hci_pci *hci;
> >>>>  	struct resource res[2];
> >>>>  	int dev_id, ret;
> >>>>
> >>>> +	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
> >>>> +	if (!hci)
> >>>> +		return -ENOMEM;
> >>>> +
> >>>>  	ret = pcim_enable_device(pci);
> >>>>  	if (ret)
> >>>>  		return ret;
> >>>> @@ -95,14 +103,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>  	if (dev_id < 0)
> >>>>  		return dev_id;
> >>>>
> >>>> -	pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> >>>> -	if (!pdev)
> >>>> +	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> >>>> +	if (!hci->pdev)
> >>>>  		return -ENOMEM;
> >>>>
> >>>> -	pdev->dev.parent = &pci->dev;
> >>>> -	device_set_node(&pdev->dev, dev_fwnode(&pci->dev));
> >>>> +	hci->pdev->dev.parent = &pci->dev;
> >>>> +	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
> >>>>
> >>>> -	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
> >>>> +	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
> >>>>  	if (ret)
> >>>>  		goto err;
> >>>>
> >>>> @@ -113,23 +121,24 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>  			goto err;
> >>>>  	}
> >>>>
> >>>> -	ret = platform_device_add(pdev);
> >>>> +	ret = platform_device_add(hci->pdev);
> >>>>  	if (ret)
> >>>>  		goto err;
> >>>>
> >>>> -	pci_set_drvdata(pci, pdev);
> >>>> +	pci_set_drvdata(pci, hci);
> >>>>
> >>>>  	return 0;
> >>>>
> >>>>  err:
> >>>> -	platform_device_put(pdev);
> >>>> +	platform_device_put(hci->pdev);
> >>>>  	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
> >>>>  	return ret;
> >>>>  }
> >>>>
> >>>>  static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
> >>>>  {
> >>>> -	struct platform_device *pdev = pci_get_drvdata(pci);
> >>>> +	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
> >>>> +	struct platform_device *pdev = hci->pdev;
> >>>>  	int dev_id = pdev->id;
> >>>>
> >>>>  	platform_device_unregister(pdev);
> >>>> --
> >>>> 2.51.0
> >>>>
> >>>>
> >>>> --
> >>>> linux-i3c mailing list
> >>>> linux-i3c@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 09/13] i3c: mipi-i3c-hci-pci: Change callback parameter

[Adrian Hunter]

Change ->init() callback parameter from a pointer to the PCI device to a
pointer to the locally defined mipi_i3c_hci_pci_info device information.
This is in preparation for consistency when adding more callbacks that will
need struct mipi_i3c_hci_pci_info.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 9fa89af7479f..5142dfcebe7d 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -15,11 +15,12 @@
 #include <linux/platform_device.h>
 
 struct mipi_i3c_hci_pci {
+	struct pci_dev *pci;
 	struct platform_device *pdev;
 };
 
 struct mipi_i3c_hci_pci_info {
-	int (*init)(struct pci_dev *pci);
+	int (*init)(struct mipi_i3c_hci_pci *hci);
 };
 
 static DEFINE_IDA(mipi_i3c_hci_pci_ida);
@@ -50,15 +51,15 @@ static void intel_reset(void __iomem *priv)
 	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
 }
 
-static int intel_init(struct pci_dev *pci)
+static int intel_init(struct mipi_i3c_hci_pci *hci)
 {
-	void __iomem *priv = intel_priv(pci);
+	void __iomem *priv = intel_priv(hci->pci);
 	int ret;
 
 	if (!priv)
 		return -ENOMEM;
 
-	ret = dma_set_mask_and_coherent(&pci->dev, DMA_BIT_MASK(64));
+	ret = dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
 	if (ret)
 		return ret;
 
@@ -83,6 +84,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	if (!hci)
 		return -ENOMEM;
 
+	hci->pci = pci;
+
 	ret = pcim_enable_device(pci);
 	if (ret)
 		return ret;
@@ -116,7 +119,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
 	if (info && info->init) {
-		ret = info->init(pci);
+		ret = info->init(hci);
 		if (ret)
 			goto err;
 	}
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 10/13] i3c: mipi-i3c-hci-pci: Add exit callback

[Adrian Hunter]

Add ->exit() callback as a counterpart to ->init().  This is in
preparation for adding device-specific features that require cleanup
upon driver removal.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 5142dfcebe7d..4209e51257b3 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -17,10 +17,12 @@
 struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
 	struct platform_device *pdev;
+	const struct mipi_i3c_hci_pci_info *info;
 };
 
 struct mipi_i3c_hci_pci_info {
 	int (*init)(struct mipi_i3c_hci_pci *hci);
+	void (*exit)(struct mipi_i3c_hci_pci *hci);
 };
 
 static DEFINE_IDA(mipi_i3c_hci_pci_ida);
@@ -75,7 +77,6 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 				  const struct pci_device_id *id)
 {
-	const struct mipi_i3c_hci_pci_info *info;
 	struct mipi_i3c_hci_pci *hci;
 	struct resource res[2];
 	int dev_id, ret;
@@ -117,9 +118,9 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	if (ret)
 		goto err;
 
-	info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
-	if (info && info->init) {
-		ret = info->init(hci);
+	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
+	if (hci->info && hci->info->init) {
+		ret = hci->info->init(hci);
 		if (ret)
 			goto err;
 	}
@@ -144,6 +145,9 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 	struct platform_device *pdev = hci->pdev;
 	int dev_id = pdev->id;
 
+	if (hci->info && hci->info->exit)
+		hci->info->exit(hci);
+
 	platform_device_unregister(pdev);
 	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
 }
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 11/13] i3c: mipi-i3c-hci-pci: Allocate a structure for Intel controller information

[Adrian Hunter]

Allocate a structure for Intel controller information, in preparation
additional changes that need to store Intel device-specific information.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 4209e51257b3..d53e9dfc5ceb 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -18,6 +18,7 @@ struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
 	struct platform_device *pdev;
 	const struct mipi_i3c_hci_pci_info *info;
+	void *private;
 };
 
 struct mipi_i3c_hci_pci_info {
@@ -34,6 +35,10 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
 #define INTEL_RESETS_RESET_DONE		BIT(1)
 #define INTEL_RESETS_TIMEOUT_US		10000
 
+struct intel_host {
+	void __iomem	*priv;
+};
+
 static void __iomem *intel_priv(struct pci_dev *pci)
 {
 	resource_size_t base = pci_resource_start(pci, 0);
@@ -55,16 +60,20 @@ static void intel_reset(void __iomem *priv)
 
 static int intel_init(struct mipi_i3c_hci_pci *hci)
 {
+	struct intel_host *host = devm_kzalloc(&hci->pci->dev, sizeof(*host), GFP_KERNEL);
 	void __iomem *priv = intel_priv(hci->pci);
 	int ret;
 
-	if (!priv)
+	if (!host || !priv)
 		return -ENOMEM;
 
 	ret = dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
 	if (ret)
 		return ret;
 
+	hci->private = host;
+	host->priv = priv;
+
 	intel_reset(priv);
 
 	return 0;
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 12/13] i3c: mipi-i3c-hci-pci: Add LTR support for Intel controllers

[Adrian Hunter]

Add support for Latency Tolerance Reporting (LTR) for Intel controllers.

Implement PM ->set_latency_tolerance() callback to set LTR register values.
Also expose LTR register values via debugfs.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index d53e9dfc5ceb..1d76dc566ab6 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -7,12 +7,14 @@
  * Author: Jarkko Nikula <jarkko.nikula@linux.intel.com>
  */
 #include <linux/acpi.h>
+#include <linux/debugfs.h>
 #include <linux/idr.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/pm_qos.h>
 
 struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
@@ -35,10 +37,118 @@ static DEFINE_IDA(mipi_i3c_hci_pci_ida);
 #define INTEL_RESETS_RESET_DONE		BIT(1)
 #define INTEL_RESETS_TIMEOUT_US		10000
 
+#define INTEL_ACTIVELTR			0x0c
+#define INTEL_IDLELTR			0x10
+
+#define INTEL_LTR_REQ			BIT(15)
+#define INTEL_LTR_SCALE_MASK		GENMASK(11, 10)
+#define INTEL_LTR_SCALE_1US		(2 << 10)
+#define INTEL_LTR_SCALE_32US		(3 << 10)
+#define INTEL_LTR_VALUE_MASK		GENMASK(9, 0)
+
 struct intel_host {
 	void __iomem	*priv;
+	u32		active_ltr;
+	u32		idle_ltr;
+	struct dentry	*debugfs_root;
 };
 
+static void intel_cache_ltr(struct intel_host *host)
+{
+	host->active_ltr = readl(host->priv + INTEL_ACTIVELTR);
+	host->idle_ltr = readl(host->priv + INTEL_IDLELTR);
+}
+
+static void intel_ltr_set(struct device *dev, s32 val)
+{
+	struct mipi_i3c_hci_pci *hci = dev_get_drvdata(dev);
+	struct intel_host *host = hci->private;
+	u32 ltr;
+
+	/*
+	 * Program latency tolerance (LTR) accordingly what has been asked
+	 * by the PM QoS layer or disable it in case we were passed
+	 * negative value or PM_QOS_LATENCY_ANY.
+	 */
+	ltr = readl(host->priv + INTEL_ACTIVELTR);
+
+	if (val == PM_QOS_LATENCY_ANY || val < 0) {
+		ltr &= ~INTEL_LTR_REQ;
+	} else {
+		ltr |= INTEL_LTR_REQ;
+		ltr &= ~INTEL_LTR_SCALE_MASK;
+		ltr &= ~INTEL_LTR_VALUE_MASK;
+
+		if (val > INTEL_LTR_VALUE_MASK)
+			ltr |= INTEL_LTR_SCALE_32US | val >> 5;
+		else
+			ltr |= INTEL_LTR_SCALE_1US | val;
+	}
+
+	if (ltr == host->active_ltr)
+		return;
+
+	writel(ltr, host->priv + INTEL_ACTIVELTR);
+	writel(ltr, host->priv + INTEL_IDLELTR);
+
+	/* Cache the values into intel_host structure */
+	intel_cache_ltr(host);
+}
+
+static void intel_ltr_expose(struct device *dev)
+{
+	dev->power.set_latency_tolerance = intel_ltr_set;
+	dev_pm_qos_expose_latency_tolerance(dev);
+}
+
+static void intel_ltr_hide(struct device *dev)
+{
+	dev_pm_qos_hide_latency_tolerance(dev);
+	dev->power.set_latency_tolerance = NULL;
+}
+
+static struct dentry *intel_actualize_debugfs_root(bool add)
+{
+	static struct dentry *debugfs_root;
+	static DEFINE_MUTEX(lock);
+	static int ref_cnt;
+
+	guard(mutex)(&lock);
+
+	ref_cnt += add ? 1 : -1;
+
+	if (ref_cnt) {
+		if (IS_ERR_OR_NULL(debugfs_root))
+			debugfs_root = debugfs_create_dir("intel_i3c", NULL);
+	} else {
+		debugfs_remove(debugfs_root);
+		debugfs_root = NULL;
+	}
+
+	return debugfs_root;
+}
+
+static void intel_add_debugfs(struct mipi_i3c_hci_pci *hci)
+{
+	struct dentry *debugfs_root = intel_actualize_debugfs_root(true);
+	struct dentry *dir = debugfs_create_dir(dev_name(&hci->pci->dev), debugfs_root);
+	struct intel_host *host = hci->private;
+
+	intel_cache_ltr(host);
+
+	host->debugfs_root = dir;
+	debugfs_create_x32("active_ltr", 0444, dir, &host->active_ltr);
+	debugfs_create_x32("idle_ltr", 0444, dir, &host->idle_ltr);
+}
+
+static void intel_remove_debugfs(struct mipi_i3c_hci_pci *hci)
+{
+	struct intel_host *host = hci->private;
+
+	debugfs_remove_recursive(host->debugfs_root);
+	intel_actualize_debugfs_root(false);
+}
+
 static void __iomem *intel_priv(struct pci_dev *pci)
 {
 	resource_size_t base = pci_resource_start(pci, 0);
@@ -76,11 +186,21 @@ static int intel_init(struct mipi_i3c_hci_pci *hci)
 
 	intel_reset(priv);
 
+	intel_ltr_expose(&hci->pci->dev);
+	intel_add_debugfs(hci);
+
 	return 0;
 }
 
+static void intel_exit(struct mipi_i3c_hci_pci *hci)
+{
+	intel_remove_debugfs(hci);
+	intel_ltr_hide(&hci->pci->dev);
+}
+
 static const struct mipi_i3c_hci_pci_info intel_info = {
 	.init = intel_init,
+	.exit = intel_exit,
 };
 
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 13/13] i3c: mipi-i3c-hci-pci: Set d3cold_delay to 0 for Intel controllers

[Adrian Hunter]

Set d3cold_delay to 0 for Intel controllers because a delay is not needed.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 1d76dc566ab6..72000d9a5fe5 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -181,6 +181,8 @@ static int intel_init(struct mipi_i3c_hci_pci *hci)
 	if (ret)
 		return ret;
 
+	hci->pci->d3cold_delay = 0;
+
 	hci->private = host;
 	host->priv = priv;
 
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c