diff mbox series

[2/7] soc: qcom: geni: move struct geni_wrapper to header

Message ID 20210111151651.1616813-3-vkoul@kernel.org (mailing list archive)
State New, archived
Headers show
Series Add and enable GPI DMA users | expand

Commit Message

Vinod Koul Jan. 11, 2021, 3:16 p.m. UTC
I2C geni driver needs to access struct geni_wrapper, so move it to
header.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/soc/qcom/qcom-geni-se.c | 15 ---------------
 include/linux/qcom-geni-se.h    | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 15 deletions(-)

Comments

Bjorn Andersson Jan. 11, 2021, 3:34 p.m. UTC | #1
On Mon 11 Jan 09:16 CST 2021, Vinod Koul wrote:

> I2C geni driver needs to access struct geni_wrapper, so move it to
> header.
> 

Please tell me more!

Glanced through the other patches and the only user I can find it in
patch 5 where you use this to get the struct device * of the wrapper.

At least in the DT case this would be [SE]->dev->parent, perhaps we
can't rely on this due to ACPI?

Regards,
Bjorn

> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  drivers/soc/qcom/qcom-geni-se.c | 15 ---------------
>  include/linux/qcom-geni-se.h    | 15 +++++++++++++++
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 285ed86c2bab..a3868228ea05 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -79,21 +79,6 @@
>   */
>  
>  #define MAX_CLK_PERF_LEVEL 32
> -#define NUM_AHB_CLKS 2
> -
> -/**
> - * struct geni_wrapper - Data structure to represent the QUP Wrapper Core
> - * @dev:		Device pointer of the QUP wrapper core
> - * @base:		Base address of this instance of QUP wrapper core
> - * @ahb_clks:		Handle to the primary & secondary AHB clocks
> - * @to_core:		Core ICC path
> - */
> -struct geni_wrapper {
> -	struct device *dev;
> -	void __iomem *base;
> -	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> -	struct geni_icc_path to_core;
> -};
>  
>  static const char * const icc_path_names[] = {"qup-core", "qup-config",
>  						"qup-memory"};
> diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
> index e3f4b16040d9..cb4e40908f9f 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -38,6 +38,21 @@ struct geni_icc_path {
>  	unsigned int avg_bw;
>  };
>  
> +#define NUM_AHB_CLKS 2
> +
> +/**
> + * @struct geni_wrapper - Data structure to represent the QUP Wrapper Core
> + * @dev:		Device pointer of the QUP wrapper core
> + * @base:		Base address of this instance of QUP wrapper core
> + * @ahb_clks:		Handle to the primary & secondary AHB clocks
> + */
> +struct geni_wrapper {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> +	struct geni_icc_path to_core;
> +};
> +
>  /**
>   * struct geni_se - GENI Serial Engine
>   * @base:		Base Address of the Serial Engine's register block
> -- 
> 2.26.2
>
Vinod Koul Jan. 11, 2021, 5:43 p.m. UTC | #2
On 11-01-21, 09:34, Bjorn Andersson wrote:
> On Mon 11 Jan 09:16 CST 2021, Vinod Koul wrote:
> 
> > I2C geni driver needs to access struct geni_wrapper, so move it to
> > header.
> > 
> 
> Please tell me more!
> 
> Glanced through the other patches and the only user I can find it in
> patch 5 where you use this to get the struct device * of the wrapper.

That is correct. The dma mapping needs to be done with SE device.

> At least in the DT case this would be [SE]->dev->parent, perhaps we
> can't rely on this due to ACPI?

I would have missed that then, but I somehow recall trying that.. Though
I have not looked into ACPI..

Given that we would need to worry about ACPI, do you recommend using
parent or keeping this
Bjorn Andersson Jan. 11, 2021, 6:51 p.m. UTC | #3
On Mon 11 Jan 11:43 CST 2021, Vinod Koul wrote:

> On 11-01-21, 09:34, Bjorn Andersson wrote:
> > On Mon 11 Jan 09:16 CST 2021, Vinod Koul wrote:
> > 
> > > I2C geni driver needs to access struct geni_wrapper, so move it to
> > > header.
> > > 
> > 
> > Please tell me more!
> > 
> > Glanced through the other patches and the only user I can find it in
> > patch 5 where you use this to get the struct device * of the wrapper.
> 
> That is correct. The dma mapping needs to be done with SE device.
> 
> > At least in the DT case this would be [SE]->dev->parent, perhaps we
> > can't rely on this due to ACPI?
> 
> I would have missed that then, but I somehow recall trying that.. Though
> I have not looked into ACPI..
> 
> Given that we would need to worry about ACPI, do you recommend using
> parent or keeping this
> 

We get the wrapper by the means of dev_drv_getdata(dev->parent),
so afaict we need to figure out how to get hold of the wrapper for ACPI
to work either way.

We then need to lug around the wrapper's device for your uses and
exposing the wrapper struct solves this for us. So I'm okay with your
proposal.

Regards,
Bjorn
diff mbox series

Patch

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 285ed86c2bab..a3868228ea05 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -79,21 +79,6 @@ 
  */
 
 #define MAX_CLK_PERF_LEVEL 32
-#define NUM_AHB_CLKS 2
-
-/**
- * struct geni_wrapper - Data structure to represent the QUP Wrapper Core
- * @dev:		Device pointer of the QUP wrapper core
- * @base:		Base address of this instance of QUP wrapper core
- * @ahb_clks:		Handle to the primary & secondary AHB clocks
- * @to_core:		Core ICC path
- */
-struct geni_wrapper {
-	struct device *dev;
-	void __iomem *base;
-	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
-	struct geni_icc_path to_core;
-};
 
 static const char * const icc_path_names[] = {"qup-core", "qup-config",
 						"qup-memory"};
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index e3f4b16040d9..cb4e40908f9f 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -38,6 +38,21 @@  struct geni_icc_path {
 	unsigned int avg_bw;
 };
 
+#define NUM_AHB_CLKS 2
+
+/**
+ * @struct geni_wrapper - Data structure to represent the QUP Wrapper Core
+ * @dev:		Device pointer of the QUP wrapper core
+ * @base:		Base address of this instance of QUP wrapper core
+ * @ahb_clks:		Handle to the primary & secondary AHB clocks
+ */
+struct geni_wrapper {
+	struct device *dev;
+	void __iomem *base;
+	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
+	struct geni_icc_path to_core;
+};
+
 /**
  * struct geni_se - GENI Serial Engine
  * @base:		Base Address of the Serial Engine's register block