[U-Boot,v0,5/5] spi: Add support for Armada 38x second controller

From: Dirk Eibach <dirk.eibach@gdsys.cc>

Armada 38x has two spi controllers.

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
---

 drivers/spi/kirkwood_spi.c | 52 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

On 28 October 2015 at 21:14,  <dirk.eibach@gdsys.cc> wrote:
> From: Dirk Eibach <dirk.eibach@gdsys.cc>
>
> Armada 38x has two spi controllers.
>
> Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
> ---
>
>  drivers/spi/kirkwood_spi.c | 52 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index e7b0982..200c391 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -18,17 +18,25 @@
>  #endif
>  #include <asm/arch-mvebu/spi.h>
>
> -static struct kwspi_registers *spireg =
> -       (struct kwspi_registers *)MVEBU_SPI_BASE;
> -
>  #ifdef CONFIG_KIRKWOOD
>  static u32 cs_spi_mpp_back[2];
>  #endif
>
> +struct kwspi_slave {
> +       struct spi_slave slave;
> +       struct kwspi_registers *spireg;
> +};
> +
> +static inline struct kwspi_slave *to_kwspi(struct spi_slave *slave)
> +{
> +       return container_of(slave, struct kwspi_slave, slave);
> +}
> +
>  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>                                 unsigned int max_hz, unsigned int mode)
>  {
> -       struct spi_slave *slave;
> +       struct kwspi_slave *kwspi_slave;
> +       struct kwspi_registers *spireg;
>         u32 data;
>  #ifdef CONFIG_KIRKWOOD
>         static const u32 kwspi_mpp_config[2][2] = {
> @@ -40,10 +48,27 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>         if (!spi_cs_is_valid(bus, cs))
>                 return NULL;
>
> -       slave = spi_alloc_slave_base(bus, cs);
> -       if (!slave)
> +       kwspi_slave = spi_alloc_slave(struct kwspi_slave, bus, cs);
> +       if (!kwspi_slave)
>                 return NULL;
>
> +       switch (bus) {
> +       case 0:
> +               kwspi_slave->spireg = (struct kwspi_registers *)MVEBU_SPI_BASE;
> +               break;
> +#ifdef CONFIG_ARMADA_38X
> +       /* Armada 38x has two SPI controllers */

Can you please add this through driver-model, I understand it's bit
big task but I can help you at any moment.

> +       case 1:
> +               kwspi_slave->spireg =
> +                       (struct kwspi_registers *)(MVEBU_SPI_BASE + 0x80);
> +               break;
> +#endif
> +       default:
> +               return NULL;
> +       }
> +
> +       spireg = kwspi_slave->spireg;
> +
>         writel(KWSPI_SMEMRDY, &spireg->ctrl);
>
>         /* calculate spi clock prescaller using max_hz */
> @@ -63,7 +88,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>         kirkwood_mpp_conf(kwspi_mpp_config[cs ? 1 : 0], cs_spi_mpp_back);
>  #endif
>
> -       return slave;
> +       return &kwspi_slave->slave;
>  }
>
>  void spi_free_slave(struct spi_slave *slave)
> @@ -137,7 +162,12 @@ void spi_release_bus(struct spi_slave *slave)
>   */
>  int spi_cs_is_valid(unsigned int bus, unsigned int cs)
>  {
> +#ifdef CONFIG_ARMADA_38X
> +       /* Armada 38x has two SPI controllers */
> +       return (bus < 2) && (cs < 3);
> +#else
>         return bus == 0 && (cs == 0 || cs == 1);
> +#endif
>  }
>  #endif
>
> @@ -147,11 +177,17 @@ void spi_init(void)
>
>  void spi_cs_activate(struct spi_slave *slave)
>  {
> +       struct kwspi_slave *kwspi_slave = to_kwspi(slave);
> +       struct kwspi_registers *spireg = kwspi_slave->spireg;
> +
>         setbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);
>  }
>
>  void spi_cs_deactivate(struct spi_slave *slave)
>  {
> +       struct kwspi_slave *kwspi_slave = to_kwspi(slave);
> +       struct kwspi_registers *spireg = kwspi_slave->spireg;
> +
>         clrbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);
>  }
>
> @@ -160,6 +196,8 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>  {
>         unsigned int tmpdout, tmpdin;
>         int tm, isread = 0;
> +       struct kwspi_slave *kwspi_slave = to_kwspi(slave);
> +       struct kwspi_registers *spireg = kwspi_slave->spireg;
>
>         debug("spi_xfer: slave %u:%u dout %p din %p bitlen %u\n",
>               slave->bus, slave->cs, dout, din, bitlen);
> --
> 2.1.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Hi Dirk,

On 28.10.2015 17:29, Jagan Teki wrote:
> On 28 October 2015 at 21:14,  <dirk.eibach@gdsys.cc> wrote:
>> From: Dirk Eibach <dirk.eibach@gdsys.cc>
>>
>> Armada 38x has two spi controllers.
>>
>> Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
>> ---
>>
>>   drivers/spi/kirkwood_spi.c | 52 +++++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 45 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
>> index e7b0982..200c391 100644
>> --- a/drivers/spi/kirkwood_spi.c
>> +++ b/drivers/spi/kirkwood_spi.c
>> @@ -18,17 +18,25 @@
>>   #endif
>>   #include <asm/arch-mvebu/spi.h>
>>
>> -static struct kwspi_registers *spireg =
>> -       (struct kwspi_registers *)MVEBU_SPI_BASE;
>> -
>>   #ifdef CONFIG_KIRKWOOD
>>   static u32 cs_spi_mpp_back[2];
>>   #endif
>>
>> +struct kwspi_slave {
>> +       struct spi_slave slave;
>> +       struct kwspi_registers *spireg;
>> +};
>> +
>> +static inline struct kwspi_slave *to_kwspi(struct spi_slave *slave)
>> +{
>> +       return container_of(slave, struct kwspi_slave, slave);
>> +}
>> +
>>   struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>>                                  unsigned int max_hz, unsigned int mode)
>>   {
>> -       struct spi_slave *slave;
>> +       struct kwspi_slave *kwspi_slave;
>> +       struct kwspi_registers *spireg;
>>          u32 data;
>>   #ifdef CONFIG_KIRKWOOD
>>          static const u32 kwspi_mpp_config[2][2] = {
>> @@ -40,10 +48,27 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>>          if (!spi_cs_is_valid(bus, cs))
>>                  return NULL;
>>
>> -       slave = spi_alloc_slave_base(bus, cs);
>> -       if (!slave)
>> +       kwspi_slave = spi_alloc_slave(struct kwspi_slave, bus, cs);
>> +       if (!kwspi_slave)
>>                  return NULL;
>>
>> +       switch (bus) {
>> +       case 0:
>> +               kwspi_slave->spireg = (struct kwspi_registers *)MVEBU_SPI_BASE;
>> +               break;
>> +#ifdef CONFIG_ARMADA_38X
>> +       /* Armada 38x has two SPI controllers */
>
> Can you please add this through driver-model, I understand it's bit
> big task but I can help you at any moment.

Yes, please do. I know this is additional work. But we really need to
get there at some time. And please note that you can use the
runtime SoC detection for this:

	if (mvebu_soc_family() == MVEBU_SOC_A38X)

So no new #idefs are needed in such places.

Thanks,
Stefan

Hi Stefan,

2015-10-28 17:39 GMT+01:00 Stefan Roese <sr@denx.de>:
> ...  And please note that you can use the
> runtime SoC detection for this:
>
>         if (mvebu_soc_family() == MVEBU_SOC_A38X)
>
> So no new #idefs are needed in such places.

Just give me a quick update please.  Why is runtime detection better?
Is it about code coverage? What about binary footprint?

Cheers
Dirk

Hi Dirk,

On 29.10.2015 10:54, Dirk Eibach wrote:
> 2015-10-28 17:39 GMT+01:00 Stefan Roese <sr@denx.de>:
>> ...  And please note that you can use the
>> runtime SoC detection for this:
>>
>>          if (mvebu_soc_family() == MVEBU_SOC_A38X)
>>
>> So no new #idefs are needed in such places.
>
> Just give me a quick update please.  Why is runtime detection better?
> Is it about code coverage? What about binary footprint?

We try hard not to add more #idef's to the U-Boot source code
if possible. This is definitely one of the reasons. Another
is, that this runtime detection will enable support for
multiple SoC's in one binary image. This is currently not the
case, but we should try to work this way if its not too
hard. And these places for the SoC detection are pretty easy
to achieve.

The image size will of course be affected. But this drawback is
outweighed by the pros noted above. At least from my point of
view.

Thanks,
Stefan

Hi Stefan,

2015-10-29 11:02 GMT+01:00 Stefan Roese <sr@denx.de>:
> Hi Dirk,
>
> On 29.10.2015 10:54, Dirk Eibach wrote:
>>
>> 2015-10-28 17:39 GMT+01:00 Stefan Roese <sr@denx.de>:
>>>
>>> ...  And please note that you can use the
>>> runtime SoC detection for this:
>>>
>>>          if (mvebu_soc_family() == MVEBU_SOC_A38X)
>>>
>>> So no new #idefs are needed in such places.
>>
>>
>> Just give me a quick update please.  Why is runtime detection better?
>> Is it about code coverage? What about binary footprint?
>
>
> We try hard not to add more #idef's to the U-Boot source code
> if possible. This is definitely one of the reasons. Another
> is, that this runtime detection will enable support for
> multiple SoC's in one binary image. This is currently not the
> case, but we should try to work this way if its not too
> hard. And these places for the SoC detection are pretty easy
> to achieve.
>
> The image size will of course be affected. But this drawback is
> outweighed by the pros noted above. At least from my point of
> view.

Ok, that's the information I needed. I will adapt this to runtime detection.

Cheers
Dirk