[6/6] i2c: Xilinx IIC: add DT Endianness support

Signed-off-by: Robert ABEL <rabel@cit-ec.uni-bielefeld.de>
---
 drivers/i2c/busses/i2c-xiic.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

On 07/31/2015 02:00 PM, Robert ABEL wrote:
> Signed-off-by: Robert ABEL <rabel@cit-ec.uni-bielefeld.de>
> ---
>  drivers/i2c/busses/i2c-xiic.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index 6a834bc..ab040a5 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -76,6 +76,8 @@ enum xilinx_i2c_reason {
>   * @state:   Current controller state
>   * @reasons: Reason for entering STATE_ERROR.
>   *           Only valid while in STATE_ERROR.
> + * @getreg32: Register Read function, respects DT endianness.
> + * @setreg32: Register Write function, respects DT endianness.
>   */
>  struct xiic_i2c {
>  	void __iomem *         base;
> @@ -87,6 +89,8 @@ struct xiic_i2c {
>  	unsigned int           nmsgs;
>  	enum xilinx_i2c_state  state;
>  	enum xilinx_i2c_reason reason;
> +	u32                    (*getreg32)(const volatile void __iomem *addr);
> +	void                   (*setreg32)(u32 value, volatile void __iomem *addr);
>  };
>  
>  
> @@ -175,8 +179,28 @@ struct xiic_i2c {
>  static void xiic_enqueue_msg(struct xiic_i2c *i2c);
>  
>  #define xiic_msg_space(i2c) ((i2c)->msg->len - (i2c)->pos)
> -#define xiic_getreg32(i2c, reg)        ioread32(i2c->base + reg)
> -#define xiic_setreg32(i2c, reg, value) iowrite32(value, i2c->base + reg)
> +#define xiic_getreg32(i2c, reg)        (i2c->getreg32(i2c->base + reg))
> +#define xiic_setreg32(i2c, reg, value) (i2c->setreg32(value, i2c->base + reg))
> +
> +static u32 xiic_getreg32le(const volatile void __iomem *addr)
> +{
> +	return ioread32(addr);
> +}
> +
> +static void xiic_setreg32le(u32 value, volatile void __iomem *addr)
> +{
> +	iowrite32(value, addr);
> +}
> +
> +static u32 xiic_getreg32be(const volatile void __iomem *addr)
> +{
> +	return ioread32be(addr);
> +}
> +
> +static void xiic_setreg32be(u32 value, volatile void __iomem *addr)
> +{
> +	iowrite32be(value, addr);
> +}
>  
>  static inline void xiic_irq_dis(struct xiic_i2c *i2c, u32 mask)
>  {
> @@ -1114,6 +1138,16 @@ static int xiic_i2c_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "Cannot claim IRQ\n");
>  		return ret;
>  	}
> +	
> +	i2c->getreg32 = xiic_getreg32le;
> +	i2c->setreg32 = xiic_setreg32le;
> +
> +#if defined(CONFIG_OF)
> +	if (of_device_is_big_endian(pdev->dev.of_node)) {
> +		i2c->getreg32 = xiic_getreg32be;
> +		i2c->setreg32 = xiic_setreg32be;
> +	}
> +#endif
>  
>  	xiic_reinit(i2c);
>  
> 

NACK for this. Previous driver version did automatic detection directly
on the IP. You are changing it to be OF driven with is error prone and
highly depends on user.

Thanks,
Michal



--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Michal,

On Mon, Aug 3, 2015 at 7:32 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> NACK for this. Previous driver version did automatic detection directly
> on the IP. You are changing it to be OF driven with is error prone and
> highly depends on user.

I beg do differ. Using the appropriate dt binary attributes is not
error prone. On the contrary, it gives the user much tighter control.

I find the solution I replaced more of a hack than anything. It writes
to reserved bits, which -- while unlikely -- might become used in
future versions of this IP.
If there was some fixed identification register, sure, this might be okay.

If you want to retain this functionality, we might put it in using
another CONFIG_I2C_XILINX_XXX option. How's that?

Regards,

Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html