[PATCH] powerpc/512x: add function for CS parameter configuration

[PATCH] powerpc/512x: add function for CS parameter configuration

[Anatolij Gustschin]

Add ability to configure CS parameters for devices that need
different CS parameters setup after their configuration. I.e.
an FPGA device on LP bus can require different CS parameters
for its bus interface after loading firmware into it. A driver
can easily reconfigure the LPC CS parameters using this function.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 arch/powerpc/include/asm/mpc5121.h           |   16 +++++++++++++
 arch/powerpc/platforms/512x/mpc512x_shared.c |   30 ++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/mpc5121.h b/arch/powerpc/include/asm/mpc5121.h
index 8c0ab2c..738ebca 100644
--- a/arch/powerpc/include/asm/mpc5121.h
+++ b/arch/powerpc/include/asm/mpc5121.h
@@ -53,4 +53,20 @@ struct mpc512x_ccm {
 	u32	m4ccr;	/* MSCAN4 CCR */
 	u8	res[0x98]; /* Reserved */
 };
+
+/*
+ * LPC Module
+ */
+struct mpc512x_lpc {
+	u32	cs_cfg[8];	/* CS config */
+	u32	cs_ctrl;	/* CS Control Register */
+	u32	cs_status;	/* CS Status Register */
+	u32	burst_ctrl;	/* CS Burst Control Register */
+	u32	deadcycle_ctrl;	/* CS Deadcycle Control Register */
+	u32	holdcycle_ctrl;	/* CS Holdcycle Control Register */
+	u32	alt;		/* Address Latch Timing Register */
+};
+
+int mpc512x_cs_config(int cs, u32 val);
+
 #endif /* __ASM_POWERPC_MPC5121_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index c344438..112b4f6 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -436,3 +436,33 @@ void __init mpc512x_init(void)
 	mpc512x_restart_init();
 	mpc512x_psc_fifo_init();
 }
+
+/**
+ * mpc512x_cs_config - Setup chip select configuration
+ * @cs: chip select number
+ * @val: chip select configuration value
+ *
+ * Perform chip select configuration for devices on LocalPlus Bus.
+ * Intended to dynamically reconfigure the chip select parameters
+ * for configurable devices on the bus.
+ */
+int mpc512x_cs_config(int cs, u32 val)
+{
+	static struct mpc512x_lpc __iomem *lpc;
+	struct device_node *np;
+
+	if (cs < 0 || cs > 7)
+		return -EINVAL;
+
+	if (!lpc) {
+		np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-lpc");
+		lpc = of_iomap(np, 0);
+		of_node_put(np);
+		if (!lpc)
+			return -ENOMEM;
+	}
+
+	out_be32(&lpc->cs_cfg[cs], val);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mpc512x_cs_config);
-- 
1.7.5.4

Re: [PATCH] powerpc/512x: add function for CS parameter configuration

[Timur Tabi]

On Fri, Feb 1, 2013 at 7:28 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Add ability to configure CS parameters for devices that need
> different CS parameters setup after their configuration. I.e.
> an FPGA device on LP bus can require different CS parameters
> for its bus interface after loading firmware into it. A driver
> can easily reconfigure the LPC CS parameters using this function.

Could you define "CS" somewhere?

> +struct mpc512x_lpc {
> +       u32     cs_cfg[8];      /* CS config */
> +       u32     cs_ctrl;        /* CS Control Register */
> +       u32     cs_status;      /* CS Status Register */
> +       u32     burst_ctrl;     /* CS Burst Control Register */
> +       u32     deadcycle_ctrl; /* CS Deadcycle Control Register */
> +       u32     holdcycle_ctrl; /* CS Holdcycle Control Register */
> +       u32     alt;            /* Address Latch Timing Register */
> +};

These should be __be32.

> +
> +int mpc512x_cs_config(int cs, u32 val);
> +
>  #endif /* __ASM_POWERPC_MPC5121_H__ */
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> index c344438..112b4f6 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -436,3 +436,33 @@ void __init mpc512x_init(void)
>         mpc512x_restart_init();
>         mpc512x_psc_fifo_init();
>  }
> +
> +/**
> + * mpc512x_cs_config - Setup chip select configuration
> + * @cs: chip select number
> + * @val: chip select configuration value
> + *
> + * Perform chip select configuration for devices on LocalPlus Bus.
> + * Intended to dynamically reconfigure the chip select parameters
> + * for configurable devices on the bus.
> + */
> +int mpc512x_cs_config(int cs, u32 val)
> +{
> +       static struct mpc512x_lpc __iomem *lpc;
> +       struct device_node *np;
> +
> +       if (cs < 0 || cs > 7)
> +               return -EINVAL;

If you make cs an unsigned int, you won't need to test for < 0.

> +
> +       if (!lpc) {
> +               np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-lpc");
> +               lpc = of_iomap(np, 0);
> +               of_node_put(np);
> +               if (!lpc)
> +                       return -ENOMEM;
> +       }
> +
> +       out_be32(&lpc->cs_cfg[cs], val);

You forgot the iounmap() if lpc == NULL.

> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(mpc512x_cs_config);

EXPORT_SYMBOL, please, to match the rest of the Freescale platforms.



-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] powerpc/512x: add function for CS parameter configuration

[Anatolij Gustschin]

On Fri, 1 Feb 2013 22:31:51 -0600
Timur Tabi <timur@tabi.org> wrote:

> On Fri, Feb 1, 2013 at 7:28 AM, Anatolij Gustschin <agust@denx.de> wrote:
> > Add ability to configure CS parameters for devices that need
> > different CS parameters setup after their configuration. I.e.
> > an FPGA device on LP bus can require different CS parameters
> > for its bus interface after loading firmware into it. A driver
> > can easily reconfigure the LPC CS parameters using this function.
> 
> Could you define "CS" somewhere?

I'll define it in revised commit log in v2.

> > +struct mpc512x_lpc {
> > +       u32     cs_cfg[8];      /* CS config */
> > +       u32     cs_ctrl;        /* CS Control Register */
> > +       u32     cs_status;      /* CS Status Register */
> > +       u32     burst_ctrl;     /* CS Burst Control Register */
> > +       u32     deadcycle_ctrl; /* CS Deadcycle Control Register */
> > +       u32     holdcycle_ctrl; /* CS Holdcycle Control Register */
> > +       u32     alt;            /* Address Latch Timing Register */
> > +};
> 
> These should be __be32.

Why? To add a new bunch of sparse warnings?

...
> > +       if (cs < 0 || cs > 7)
> > +               return -EINVAL;
> 
> If you make cs an unsigned int, you won't need to test for < 0.

can do that, yes.

> > +
> > +       if (!lpc) {
> > +               np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-lpc");
> > +               lpc = of_iomap(np, 0);
> > +               of_node_put(np);
> > +               if (!lpc)
> > +                       return -ENOMEM;
> > +       }
> > +
> > +       out_be32(&lpc->cs_cfg[cs], val);
> 
> You forgot the iounmap() if lpc == NULL.

No, it is intentional, no need to map/unmap again and again for all
subsequent calls.


> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(mpc512x_cs_config);
> 
> EXPORT_SYMBOL, please, to match the rest of the Freescale platforms.

I'll change it in v2. Thanks!

Anatolij

Re: [PATCH] powerpc/512x: add function for CS parameter configuration

[Timur Tabi]

Anatolij Gustschin wrote:

>>> +struct mpc512x_lpc {
>>> +       u32     cs_cfg[8];      /* CS config */
>>> +       u32     cs_ctrl;        /* CS Control Register */
>>> +       u32     cs_status;      /* CS Status Register */
>>> +       u32     burst_ctrl;     /* CS Burst Control Register */
>>> +       u32     deadcycle_ctrl; /* CS Deadcycle Control Register */
>>> +       u32     holdcycle_ctrl; /* CS Holdcycle Control Register */
>>> +       u32     alt;            /* Address Latch Timing Register */
>>> +};
>>
>> These should be __be32.
>
> Why? To add a new bunch of sparse warnings?

Hmm... I thought that making them __be32 will *avoid* sparse warnings.

>> You forgot the iounmap() if lpc == NULL.
>
> No, it is intentional, no need to map/unmap again and again for all
> subsequent calls.

Sorry, for some reason I thought that lpc was a parameter that you passed in.

-- 
Timur Tabi

[PATCH v2] powerpc/512x: add function for chip select parameter configuration

[Anatolij Gustschin]

Add ability to configure chip select (CS) parameters for devices
that need different CS parameters setup after their configuration.
I.e. an FPGA device on LP bus can require different CS parameters
for its bus interface after loading firmware into it. A driver
can easily reconfigure the LPC CS parameters using this function.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - define CS in commit log
 - use unsigned int type for cs argument to simplify
   valid CS range check
 - use EXPORT_SYMBOL

 arch/powerpc/include/asm/mpc5121.h           |   16 +++++++++++++
 arch/powerpc/platforms/512x/mpc512x_shared.c |   30 ++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/mpc5121.h b/arch/powerpc/include/asm/mpc5121.h
index 8c0ab2c..8ae133e 100644
--- a/arch/powerpc/include/asm/mpc5121.h
+++ b/arch/powerpc/include/asm/mpc5121.h
@@ -53,4 +53,20 @@ struct mpc512x_ccm {
 	u32	m4ccr;	/* MSCAN4 CCR */
 	u8	res[0x98]; /* Reserved */
 };
+
+/*
+ * LPC Module
+ */
+struct mpc512x_lpc {
+	u32	cs_cfg[8];	/* CS config */
+	u32	cs_ctrl;	/* CS Control Register */
+	u32	cs_status;	/* CS Status Register */
+	u32	burst_ctrl;	/* CS Burst Control Register */
+	u32	deadcycle_ctrl;	/* CS Deadcycle Control Register */
+	u32	holdcycle_ctrl;	/* CS Holdcycle Control Register */
+	u32	alt;		/* Address Latch Timing Register */
+};
+
+int mpc512x_cs_config(unsigned int cs, u32 val);
+
 #endif /* __ASM_POWERPC_MPC5121_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index 39d5630..8e2bde9 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -456,3 +456,33 @@ void __init mpc512x_init(void)
 	mpc512x_restart_init();
 	mpc512x_psc_fifo_init();
 }
+
+/**
+ * mpc512x_cs_config - Setup chip select configuration
+ * @cs: chip select number
+ * @val: chip select configuration value
+ *
+ * Perform chip select configuration for devices on LocalPlus Bus.
+ * Intended to dynamically reconfigure the chip select parameters
+ * for configurable devices on the bus.
+ */
+int mpc512x_cs_config(unsigned int cs, u32 val)
+{
+	static struct mpc512x_lpc __iomem *lpc;
+	struct device_node *np;
+
+	if (cs > 7)
+		return -EINVAL;
+
+	if (!lpc) {
+		np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-lpc");
+		lpc = of_iomap(np, 0);
+		of_node_put(np);
+		if (!lpc)
+			return -ENOMEM;
+	}
+
+	out_be32(&lpc->cs_cfg[cs], val);
+	return 0;
+}
+EXPORT_SYMBOL(mpc512x_cs_config);
-- 
1.7.5.4

Re: [PATCH v2] powerpc/512x: add function for chip select parameter configuration

[Timur Tabi]

On Mon, Feb 4, 2013 at 4:16 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Add ability to configure chip select (CS) parameters for devices
> that need different CS parameters setup after their configuration.
> I.e. an FPGA device on LP bus can require different CS parameters
> for its bus interface after loading firmware into it. A driver
> can easily reconfigure the LPC CS parameters using this function.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---

Acked-by: Timur Tabi <timur@tabi.org>

-- 
Timur Tabi
Linux kernel developer at Freescale