diff mbox series

[U-Boot,1/2] arm: sunxi: allwinner spi driver sun6i support

Message ID 20190105175216.7086-2-oskari@lemmela.net
State Rejected
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series sunxi sun6i SPI support | expand

Commit Message

Oskari Lemmelä Jan. 5, 2019, 5:52 p.m. UTC
Minimal changes to support sun6i based Allwinner SOCs
Changes are based to SPL driver arch/arm/mach-sunxi/spl_spi_sunxi.c

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h |   1 +
 arch/arm/include/asm/arch-sunxi/clock_sun9i.h |   1 +
 drivers/spi/Kconfig                           |   4 +-
 drivers/spi/sun4i_spi.c                       | 116 ++++++++++++++++--
 4 files changed, 107 insertions(+), 15 deletions(-)

Comments

Alexander Graf Jan. 22, 2019, 2:54 p.m. UTC | #1
On 05.01.19 18:52, Oskari Lemmela wrote:
> Minimal changes to support sun6i based Allwinner SOCs
> Changes are based to SPL driver arch/arm/mach-sunxi/spl_spi_sunxi.c
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>

I just tried to see if this patch gives me "sf" access on a SoPine
system. Unfortunately, it seems as if the sun4i_spi driver doesn't even
get probed?

How did you manage to actually make use of this driver?


Alex
Oskari Lemmelä Jan. 22, 2019, 4:17 p.m. UTC | #2
Hi,

On 22.1.2019 16.54, Alexander Graf wrote:
>
> On 05.01.19 18:52, Oskari Lemmela wrote:
>> Minimal changes to support sun6i based Allwinner SOCs
>> Changes are based to SPL driver arch/arm/mach-sunxi/spl_spi_sunxi.c
>>
>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
> I just tried to see if this patch gives me "sf" access on a SoPine
> system. Unfortunately, it seems as if the sun4i_spi driver doesn't even
> get probed?
>
> How did you manage to actually make use of this driver?
>
>
> Alex

You need to add spi0 alias to dts and enable needed drivers to defconfig.
You can take look from Jagan's patch [1].

I didn't include those changes as Maxime already commented [2]
that Kconfig depends/defaults should be modified first.

Thanks,
Oskari

[1] https://lists.denx.de/pipermail/u-boot/2018-December/353241.html
[2] https://lists.denx.de/pipermail/u-boot/2019-January/353784.html
Alexander Graf Jan. 22, 2019, 4:28 p.m. UTC | #3
On 22.01.19 17:17, Oskari Lemmelä wrote:
> Hi,
> 
> On 22.1.2019 16.54, Alexander Graf wrote:
>>
>> On 05.01.19 18:52, Oskari Lemmela wrote:
>>> Minimal changes to support sun6i based Allwinner SOCs
>>> Changes are based to SPL driver arch/arm/mach-sunxi/spl_spi_sunxi.c
>>>
>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
>> I just tried to see if this patch gives me "sf" access on a SoPine
>> system. Unfortunately, it seems as if the sun4i_spi driver doesn't even
>> get probed?
>>
>> How did you manage to actually make use of this driver?
>>
>>
>> Alex
> 
> You need to add spi0 alias to dts and enable needed drivers to defconfig.
> You can take look from Jagan's patch [1].
> 
> I didn't include those changes as Maxime already commented [2]
> that Kconfig depends/defaults should be modified first.

I don't see any relation between the dts change and the Kconfig
dependency issues?

But thanks for the pointer, I'll give this patch a try.


Alex
Alexander Graf Jan. 22, 2019, 4:37 p.m. UTC | #4
On 22.01.19 17:28, Alexander Graf wrote:
> 
> 
> On 22.01.19 17:17, Oskari Lemmelä wrote:
>> Hi,
>>
>> On 22.1.2019 16.54, Alexander Graf wrote:
>>>
>>> On 05.01.19 18:52, Oskari Lemmela wrote:
>>>> Minimal changes to support sun6i based Allwinner SOCs
>>>> Changes are based to SPL driver arch/arm/mach-sunxi/spl_spi_sunxi.c
>>>>
>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
>>> I just tried to see if this patch gives me "sf" access on a SoPine
>>> system. Unfortunately, it seems as if the sun4i_spi driver doesn't even
>>> get probed?
>>>
>>> How did you manage to actually make use of this driver?
>>>
>>>
>>> Alex
>>
>> You need to add spi0 alias to dts and enable needed drivers to defconfig.
>> You can take look from Jagan's patch [1].
>>
>> I didn't include those changes as Maxime already commented [2]
>> that Kconfig depends/defaults should be modified first.
> 
> I don't see any relation between the dts change and the Kconfig
> dependency issues?
> 
> But thanks for the pointer, I'll give this patch a try.

Ok, so I now get the SPI controller initialized, but it seems to only
returns zeros when trying to access the SPI flash device.

In other words, it doesn't work for me :). But maybe I'm missing all the
other clk patches to actually get it working?


Alex
Alexander Graf Jan. 24, 2019, 10:17 a.m. UTC | #5
On 22.01.19 17:37, Alexander Graf wrote:
> 
> 
> On 22.01.19 17:28, Alexander Graf wrote:
>>
>>
>> On 22.01.19 17:17, Oskari Lemmelä wrote:
>>> Hi,
>>>
>>> On 22.1.2019 16.54, Alexander Graf wrote:
>>>>
>>>> On 05.01.19 18:52, Oskari Lemmela wrote:
>>>>> Minimal changes to support sun6i based Allwinner SOCs
>>>>> Changes are based to SPL driver arch/arm/mach-sunxi/spl_spi_sunxi.c
>>>>>
>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
>>>> I just tried to see if this patch gives me "sf" access on a SoPine
>>>> system. Unfortunately, it seems as if the sun4i_spi driver doesn't even
>>>> get probed?
>>>>
>>>> How did you manage to actually make use of this driver?
>>>>
>>>>
>>>> Alex
>>>
>>> You need to add spi0 alias to dts and enable needed drivers to defconfig.
>>> You can take look from Jagan's patch [1].
>>>
>>> I didn't include those changes as Maxime already commented [2]
>>> that Kconfig depends/defaults should be modified first.
>>
>> I don't see any relation between the dts change and the Kconfig
>> dependency issues?
>>
>> But thanks for the pointer, I'll give this patch a try.
> 
> Ok, so I now get the SPI controller initialized, but it seems to only
> returns zeros when trying to access the SPI flash device.
> 
> In other words, it doesn't work for me :). But maybe I'm missing all the
> other clk patches to actually get it working?

Ok, so turns out the problem is that you can't have eMMC and SPI both
working. I don't quite know yet whether this is just a pinmux problem
(so potentially software workaroundable) or if it's a hardware
limitation (eMMC pulling a line required for SPI).

But either way, without the eMMC module, things seem to work:

  Tested-by: Alexander Graf <agraf@suse.de>


Alex
Andre Przywara Jan. 24, 2019, 10:57 a.m. UTC | #6
On Thu, 24 Jan 2019 11:17:03 +0100
Alexander Graf <agraf@suse.de> wrote:

> On 22.01.19 17:37, Alexander Graf wrote:
> > 
> > 
> > On 22.01.19 17:28, Alexander Graf wrote:  
> >>
> >>
> >> On 22.01.19 17:17, Oskari Lemmelä wrote:  
> >>> Hi,
> >>>
> >>> On 22.1.2019 16.54, Alexander Graf wrote:  
> >>>>
> >>>> On 05.01.19 18:52, Oskari Lemmela wrote:  
> >>>>> Minimal changes to support sun6i based Allwinner SOCs
> >>>>> Changes are based to SPL driver
> >>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
> >>>>>
> >>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>  
> >>>> I just tried to see if this patch gives me "sf" access on a
> >>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
> >>>> driver doesn't even get probed?
> >>>>
> >>>> How did you manage to actually make use of this driver?
> >>>>
> >>>>
> >>>> Alex  
> >>>
> >>> You need to add spi0 alias to dts and enable needed drivers to
> >>> defconfig. You can take look from Jagan's patch [1].
> >>>
> >>> I didn't include those changes as Maxime already commented [2]
> >>> that Kconfig depends/defaults should be modified first.  
> >>
> >> I don't see any relation between the dts change and the Kconfig
> >> dependency issues?
> >>
> >> But thanks for the pointer, I'll give this patch a try.  
> > 
> > Ok, so I now get the SPI controller initialized, but it seems to
> > only returns zeros when trying to access the SPI flash device.
> > 
> > In other words, it doesn't work for me :). But maybe I'm missing
> > all the other clk patches to actually get it working?  
> 
> Ok, so turns out the problem is that you can't have eMMC and SPI both
> working. I don't quite know yet whether this is just a pinmux problem
> (so potentially software workaroundable) or if it's a hardware
> limitation (eMMC pulling a line required for SPI).

The line that is shared is "DS", which is only needed for higher (e)MMC
modes (HS-400). At the moment neither Linux nor U-Boot implement this
mode, and probably U-Boot will never need it: it adds software
complexity for a questionable performance benefit, especially if the
flash chips can't keep up with the high interface speed anyway.

We addressed this in the DTs (kernel commit fa59dd2ef755).

I am a bit puzzled here as why this happens: I can't find where U-Boot
would configure PC1 to use the MMC2 function: the hard-coded
mmc_pinmux_setup() doesn't touch PC1 at all. Maybe it's the BootROM
configuring this line? So we would need to reset it?

As I don't have an eMMC module here at hand right now, this has to wait
till tonight for further insights.

> But either way, without the eMMC module, things seem to work:
> 
>   Tested-by: Alexander Graf <agraf@suse.de>

Does anyone care to clean up the Kconfig dependency mess, since we seem
to know all the options we need now? Ideally we would just need one or
two symbols in the *_defconfig files, the rest being automatically
selected by Kconfig magic.
It's on my list, but there are other things higher on that one.

Cheers,
Andre.
Alexander Graf Jan. 24, 2019, 10:59 a.m. UTC | #7
On 24.01.19 11:57, Andre Przywara wrote:
> On Thu, 24 Jan 2019 11:17:03 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> On 22.01.19 17:37, Alexander Graf wrote:
>>>
>>>
>>> On 22.01.19 17:28, Alexander Graf wrote:  
>>>>
>>>>
>>>> On 22.01.19 17:17, Oskari Lemmelä wrote:  
>>>>> Hi,
>>>>>
>>>>> On 22.1.2019 16.54, Alexander Graf wrote:  
>>>>>>
>>>>>> On 05.01.19 18:52, Oskari Lemmela wrote:  
>>>>>>> Minimal changes to support sun6i based Allwinner SOCs
>>>>>>> Changes are based to SPL driver
>>>>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
>>>>>>>
>>>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>  
>>>>>> I just tried to see if this patch gives me "sf" access on a
>>>>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
>>>>>> driver doesn't even get probed?
>>>>>>
>>>>>> How did you manage to actually make use of this driver?
>>>>>>
>>>>>>
>>>>>> Alex  
>>>>>
>>>>> You need to add spi0 alias to dts and enable needed drivers to
>>>>> defconfig. You can take look from Jagan's patch [1].
>>>>>
>>>>> I didn't include those changes as Maxime already commented [2]
>>>>> that Kconfig depends/defaults should be modified first.  
>>>>
>>>> I don't see any relation between the dts change and the Kconfig
>>>> dependency issues?
>>>>
>>>> But thanks for the pointer, I'll give this patch a try.  
>>>
>>> Ok, so I now get the SPI controller initialized, but it seems to
>>> only returns zeros when trying to access the SPI flash device.
>>>
>>> In other words, it doesn't work for me :). But maybe I'm missing
>>> all the other clk patches to actually get it working?  
>>
>> Ok, so turns out the problem is that you can't have eMMC and SPI both
>> working. I don't quite know yet whether this is just a pinmux problem
>> (so potentially software workaroundable) or if it's a hardware
>> limitation (eMMC pulling a line required for SPI).
> 
> The line that is shared is "DS", which is only needed for higher (e)MMC
> modes (HS-400). At the moment neither Linux nor U-Boot implement this
> mode, and probably U-Boot will never need it: it adds software
> complexity for a questionable performance benefit, especially if the
> flash chips can't keep up with the high interface speed anyway.
> 
> We addressed this in the DTs (kernel commit fa59dd2ef755).
> 
> I am a bit puzzled here as why this happens: I can't find where U-Boot
> would configure PC1 to use the MMC2 function: the hard-coded
> mmc_pinmux_setup() doesn't touch PC1 at all. Maybe it's the BootROM
> configuring this line? So we would need to reset it?
> 
> As I don't have an eMMC module here at hand right now, this has to wait
> till tonight for further insights.

A few more data points:

When I boot w/o eMMC module attached, SPI flash works
When I then plug in the eMMC module, SPI flash still works
When I that start initializing the eMMC module, SPI flash fails
When I then unplug the eMMC module, SPI flash works again

So this is not pinmux related. The eMMC module really just drives the line.


Alex
Andre Przywara Jan. 24, 2019, 11:07 a.m. UTC | #8
On Thu, 24 Jan 2019 11:59:32 +0100
Alexander Graf <agraf@suse.de> wrote:

> On 24.01.19 11:57, Andre Przywara wrote:
> > On Thu, 24 Jan 2019 11:17:03 +0100
> > Alexander Graf <agraf@suse.de> wrote:
> >   
> >> On 22.01.19 17:37, Alexander Graf wrote:  
> >>>
> >>>
> >>> On 22.01.19 17:28, Alexander Graf wrote:    
> >>>>
> >>>>
> >>>> On 22.01.19 17:17, Oskari Lemmelä wrote:    
> >>>>> Hi,
> >>>>>
> >>>>> On 22.1.2019 16.54, Alexander Graf wrote:    
> >>>>>>
> >>>>>> On 05.01.19 18:52, Oskari Lemmela wrote:    
> >>>>>>> Minimal changes to support sun6i based Allwinner SOCs
> >>>>>>> Changes are based to SPL driver
> >>>>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
> >>>>>>>
> >>>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>    
> >>>>>> I just tried to see if this patch gives me "sf" access on a
> >>>>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
> >>>>>> driver doesn't even get probed?
> >>>>>>
> >>>>>> How did you manage to actually make use of this driver?
> >>>>>>
> >>>>>>
> >>>>>> Alex    
> >>>>>
> >>>>> You need to add spi0 alias to dts and enable needed drivers to
> >>>>> defconfig. You can take look from Jagan's patch [1].
> >>>>>
> >>>>> I didn't include those changes as Maxime already commented [2]
> >>>>> that Kconfig depends/defaults should be modified first.    
> >>>>
> >>>> I don't see any relation between the dts change and the Kconfig
> >>>> dependency issues?
> >>>>
> >>>> But thanks for the pointer, I'll give this patch a try.    
> >>>
> >>> Ok, so I now get the SPI controller initialized, but it seems to
> >>> only returns zeros when trying to access the SPI flash device.
> >>>
> >>> In other words, it doesn't work for me :). But maybe I'm missing
> >>> all the other clk patches to actually get it working?    
> >>
> >> Ok, so turns out the problem is that you can't have eMMC and SPI
> >> both working. I don't quite know yet whether this is just a pinmux
> >> problem (so potentially software workaroundable) or if it's a
> >> hardware limitation (eMMC pulling a line required for SPI).  
> > 
> > The line that is shared is "DS", which is only needed for higher
> > (e)MMC modes (HS-400). At the moment neither Linux nor U-Boot
> > implement this mode, and probably U-Boot will never need it: it
> > adds software complexity for a questionable performance benefit,
> > especially if the flash chips can't keep up with the high interface
> > speed anyway.
> > 
> > We addressed this in the DTs (kernel commit fa59dd2ef755).
> > 
> > I am a bit puzzled here as why this happens: I can't find where
> > U-Boot would configure PC1 to use the MMC2 function: the hard-coded
> > mmc_pinmux_setup() doesn't touch PC1 at all. Maybe it's the BootROM
> > configuring this line? So we would need to reset it?
> > 
> > As I don't have an eMMC module here at hand right now, this has to
> > wait till tonight for further insights.  
> 
> A few more data points:
> 
> When I boot w/o eMMC module attached, SPI flash works
> When I then plug in the eMMC module, SPI flash still works

Plug in with power off (which you should always do)? So it goes through
the BootROM?

> When I that start initializing the eMMC module, SPI flash fails

What do you mean by that, exactly? Accessing eMMC from U-Boot?
Because that is the interesting data point, somehow this makes the
eMMC module drive the pin.

> When I then unplug the eMMC module, SPI flash works again

Again, at runtime?

> So this is not pinmux related. The eMMC module really just drives the
> line.

Thanks for that.

Cheers,
Andre.
Alexander Graf Jan. 24, 2019, 11:21 a.m. UTC | #9
On 24.01.19 12:07, Andre Przywara wrote:
> On Thu, 24 Jan 2019 11:59:32 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> On 24.01.19 11:57, Andre Przywara wrote:
>>> On Thu, 24 Jan 2019 11:17:03 +0100
>>> Alexander Graf <agraf@suse.de> wrote:
>>>   
>>>> On 22.01.19 17:37, Alexander Graf wrote:  
>>>>>
>>>>>
>>>>> On 22.01.19 17:28, Alexander Graf wrote:    
>>>>>>
>>>>>>
>>>>>> On 22.01.19 17:17, Oskari Lemmelä wrote:    
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 22.1.2019 16.54, Alexander Graf wrote:    
>>>>>>>>
>>>>>>>> On 05.01.19 18:52, Oskari Lemmela wrote:    
>>>>>>>>> Minimal changes to support sun6i based Allwinner SOCs
>>>>>>>>> Changes are based to SPL driver
>>>>>>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
>>>>>>>>>
>>>>>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>    
>>>>>>>> I just tried to see if this patch gives me "sf" access on a
>>>>>>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
>>>>>>>> driver doesn't even get probed?
>>>>>>>>
>>>>>>>> How did you manage to actually make use of this driver?
>>>>>>>>
>>>>>>>>
>>>>>>>> Alex    
>>>>>>>
>>>>>>> You need to add spi0 alias to dts and enable needed drivers to
>>>>>>> defconfig. You can take look from Jagan's patch [1].
>>>>>>>
>>>>>>> I didn't include those changes as Maxime already commented [2]
>>>>>>> that Kconfig depends/defaults should be modified first.    
>>>>>>
>>>>>> I don't see any relation between the dts change and the Kconfig
>>>>>> dependency issues?
>>>>>>
>>>>>> But thanks for the pointer, I'll give this patch a try.    
>>>>>
>>>>> Ok, so I now get the SPI controller initialized, but it seems to
>>>>> only returns zeros when trying to access the SPI flash device.
>>>>>
>>>>> In other words, it doesn't work for me :). But maybe I'm missing
>>>>> all the other clk patches to actually get it working?    
>>>>
>>>> Ok, so turns out the problem is that you can't have eMMC and SPI
>>>> both working. I don't quite know yet whether this is just a pinmux
>>>> problem (so potentially software workaroundable) or if it's a
>>>> hardware limitation (eMMC pulling a line required for SPI).  
>>>
>>> The line that is shared is "DS", which is only needed for higher
>>> (e)MMC modes (HS-400). At the moment neither Linux nor U-Boot
>>> implement this mode, and probably U-Boot will never need it: it
>>> adds software complexity for a questionable performance benefit,
>>> especially if the flash chips can't keep up with the high interface
>>> speed anyway.
>>>
>>> We addressed this in the DTs (kernel commit fa59dd2ef755).
>>>
>>> I am a bit puzzled here as why this happens: I can't find where
>>> U-Boot would configure PC1 to use the MMC2 function: the hard-coded
>>> mmc_pinmux_setup() doesn't touch PC1 at all. Maybe it's the BootROM
>>> configuring this line? So we would need to reset it?
>>>
>>> As I don't have an eMMC module here at hand right now, this has to
>>> wait till tonight for further insights.  
>>
>> A few more data points:
>>
>> When I boot w/o eMMC module attached, SPI flash works
>> When I then plug in the eMMC module, SPI flash still works
> 
> Plug in with power off (which you should always do)? So it goes through
> the BootROM?
> 
>> When I that start initializing the eMMC module, SPI flash fails
> 
> What do you mean by that, exactly? Accessing eMMC from U-Boot?

Yes, "mmc part" to be precise.

> Because that is the interesting data point, somehow this makes the
> eMMC module drive the pin.
> 
>> When I then unplug the eMMC module, SPI flash works again
> 
> Again, at runtime?

Yup, all of it at runtime. I felt darey and ignored the fact that you
should really only plug the eMMC module at boot time ;).

The last step definitely did not change pinmux. Hence the conclusion
that this is line drive collision, not pinmux.

So I guess you're saying that forcing the eMMC into 4-DAT mode would
make things work?


Alex
Andre Przywara Jan. 24, 2019, 11:31 a.m. UTC | #10
On Thu, 24 Jan 2019 12:21:17 +0100
Alexander Graf <agraf@suse.de> wrote:

> On 24.01.19 12:07, Andre Przywara wrote:
> > On Thu, 24 Jan 2019 11:59:32 +0100
> > Alexander Graf <agraf@suse.de> wrote:
> >   
> >> On 24.01.19 11:57, Andre Przywara wrote:  
> >>> On Thu, 24 Jan 2019 11:17:03 +0100
> >>> Alexander Graf <agraf@suse.de> wrote:
> >>>     
> >>>> On 22.01.19 17:37, Alexander Graf wrote:    
> >>>>>
> >>>>>
> >>>>> On 22.01.19 17:28, Alexander Graf wrote:      
> >>>>>>
> >>>>>>
> >>>>>> On 22.01.19 17:17, Oskari Lemmelä wrote:      
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> On 22.1.2019 16.54, Alexander Graf wrote:      
> >>>>>>>>
> >>>>>>>> On 05.01.19 18:52, Oskari Lemmela wrote:      
> >>>>>>>>> Minimal changes to support sun6i based Allwinner SOCs
> >>>>>>>>> Changes are based to SPL driver
> >>>>>>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>      
> >>>>>>>> I just tried to see if this patch gives me "sf" access on a
> >>>>>>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
> >>>>>>>> driver doesn't even get probed?
> >>>>>>>>
> >>>>>>>> How did you manage to actually make use of this driver?
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Alex      
> >>>>>>>
> >>>>>>> You need to add spi0 alias to dts and enable needed drivers to
> >>>>>>> defconfig. You can take look from Jagan's patch [1].
> >>>>>>>
> >>>>>>> I didn't include those changes as Maxime already commented [2]
> >>>>>>> that Kconfig depends/defaults should be modified first.      
> >>>>>>
> >>>>>> I don't see any relation between the dts change and the Kconfig
> >>>>>> dependency issues?
> >>>>>>
> >>>>>> But thanks for the pointer, I'll give this patch a try.      
> >>>>>
> >>>>> Ok, so I now get the SPI controller initialized, but it seems to
> >>>>> only returns zeros when trying to access the SPI flash device.
> >>>>>
> >>>>> In other words, it doesn't work for me :). But maybe I'm missing
> >>>>> all the other clk patches to actually get it working?      
> >>>>
> >>>> Ok, so turns out the problem is that you can't have eMMC and SPI
> >>>> both working. I don't quite know yet whether this is just a
> >>>> pinmux problem (so potentially software workaroundable) or if
> >>>> it's a hardware limitation (eMMC pulling a line required for
> >>>> SPI).    
> >>>
> >>> The line that is shared is "DS", which is only needed for higher
> >>> (e)MMC modes (HS-400). At the moment neither Linux nor U-Boot
> >>> implement this mode, and probably U-Boot will never need it: it
> >>> adds software complexity for a questionable performance benefit,
> >>> especially if the flash chips can't keep up with the high
> >>> interface speed anyway.
> >>>
> >>> We addressed this in the DTs (kernel commit fa59dd2ef755).
> >>>
> >>> I am a bit puzzled here as why this happens: I can't find where
> >>> U-Boot would configure PC1 to use the MMC2 function: the
> >>> hard-coded mmc_pinmux_setup() doesn't touch PC1 at all. Maybe
> >>> it's the BootROM configuring this line? So we would need to reset
> >>> it?
> >>>
> >>> As I don't have an eMMC module here at hand right now, this has to
> >>> wait till tonight for further insights.    
> >>
> >> A few more data points:
> >>
> >> When I boot w/o eMMC module attached, SPI flash works
> >> When I then plug in the eMMC module, SPI flash still works  
> > 
> > Plug in with power off (which you should always do)? So it goes
> > through the BootROM?
> >   
> >> When I that start initializing the eMMC module, SPI flash fails  
> > 
> > What do you mean by that, exactly? Accessing eMMC from U-Boot?  
> 
> Yes, "mmc part" to be precise.
> 
> > Because that is the interesting data point, somehow this makes the
> > eMMC module drive the pin.
> >   
> >> When I then unplug the eMMC module, SPI flash works again  
> > 
> > Again, at runtime?  
> 
> Yup, all of it at runtime. I felt darey and ignored the fact that you
> should really only plug the eMMC module at boot time ;).
> 
> The last step definitely did not change pinmux. Hence the conclusion
> that this is line drive collision, not pinmux.
> 
> So I guess you're saying that forcing the eMMC into 4-DAT mode would
> make things work?

Technically DS is for HS-400, which is a step beyond 8-bit bus width.
So just enabling 8 bit should not affect the DS line. There is a bit in
the MMC controller to enable HS-400, but I don't see that we would set
it (we actually don't even define the register).
You could try to force eMMC to 4 bit to check this theory, by
hack-removing the MMC_MODE_8BIT line in sunxi_mmc_init() in
drivers/mmc/sunxi-mmc.c (we don't use DM_MMC yet, so it's in the hacky
part and ignores the DT property)

Cheers,
Andre.
Alexander Graf Jan. 24, 2019, 12:07 p.m. UTC | #11
On 24.01.19 12:31, Andre Przywara wrote:
> On Thu, 24 Jan 2019 12:21:17 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> On 24.01.19 12:07, Andre Przywara wrote:
>>> On Thu, 24 Jan 2019 11:59:32 +0100
>>> Alexander Graf <agraf@suse.de> wrote:
>>>   
>>>> On 24.01.19 11:57, Andre Przywara wrote:  
>>>>> On Thu, 24 Jan 2019 11:17:03 +0100
>>>>> Alexander Graf <agraf@suse.de> wrote:
>>>>>     
>>>>>> On 22.01.19 17:37, Alexander Graf wrote:    
>>>>>>>
>>>>>>>
>>>>>>> On 22.01.19 17:28, Alexander Graf wrote:      
>>>>>>>>
>>>>>>>>
>>>>>>>> On 22.01.19 17:17, Oskari Lemmelä wrote:      
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 22.1.2019 16.54, Alexander Graf wrote:      
>>>>>>>>>>
>>>>>>>>>> On 05.01.19 18:52, Oskari Lemmela wrote:      
>>>>>>>>>>> Minimal changes to support sun6i based Allwinner SOCs
>>>>>>>>>>> Changes are based to SPL driver
>>>>>>>>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>      
>>>>>>>>>> I just tried to see if this patch gives me "sf" access on a
>>>>>>>>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
>>>>>>>>>> driver doesn't even get probed?
>>>>>>>>>>
>>>>>>>>>> How did you manage to actually make use of this driver?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Alex      
>>>>>>>>>
>>>>>>>>> You need to add spi0 alias to dts and enable needed drivers to
>>>>>>>>> defconfig. You can take look from Jagan's patch [1].
>>>>>>>>>
>>>>>>>>> I didn't include those changes as Maxime already commented [2]
>>>>>>>>> that Kconfig depends/defaults should be modified first.      
>>>>>>>>
>>>>>>>> I don't see any relation between the dts change and the Kconfig
>>>>>>>> dependency issues?
>>>>>>>>
>>>>>>>> But thanks for the pointer, I'll give this patch a try.      
>>>>>>>
>>>>>>> Ok, so I now get the SPI controller initialized, but it seems to
>>>>>>> only returns zeros when trying to access the SPI flash device.
>>>>>>>
>>>>>>> In other words, it doesn't work for me :). But maybe I'm missing
>>>>>>> all the other clk patches to actually get it working?      
>>>>>>
>>>>>> Ok, so turns out the problem is that you can't have eMMC and SPI
>>>>>> both working. I don't quite know yet whether this is just a
>>>>>> pinmux problem (so potentially software workaroundable) or if
>>>>>> it's a hardware limitation (eMMC pulling a line required for
>>>>>> SPI).    
>>>>>
>>>>> The line that is shared is "DS", which is only needed for higher
>>>>> (e)MMC modes (HS-400). At the moment neither Linux nor U-Boot
>>>>> implement this mode, and probably U-Boot will never need it: it
>>>>> adds software complexity for a questionable performance benefit,
>>>>> especially if the flash chips can't keep up with the high
>>>>> interface speed anyway.
>>>>>
>>>>> We addressed this in the DTs (kernel commit fa59dd2ef755).
>>>>>
>>>>> I am a bit puzzled here as why this happens: I can't find where
>>>>> U-Boot would configure PC1 to use the MMC2 function: the
>>>>> hard-coded mmc_pinmux_setup() doesn't touch PC1 at all. Maybe
>>>>> it's the BootROM configuring this line? So we would need to reset
>>>>> it?
>>>>>
>>>>> As I don't have an eMMC module here at hand right now, this has to
>>>>> wait till tonight for further insights.    
>>>>
>>>> A few more data points:
>>>>
>>>> When I boot w/o eMMC module attached, SPI flash works
>>>> When I then plug in the eMMC module, SPI flash still works  
>>>
>>> Plug in with power off (which you should always do)? So it goes
>>> through the BootROM?
>>>   
>>>> When I that start initializing the eMMC module, SPI flash fails  
>>>
>>> What do you mean by that, exactly? Accessing eMMC from U-Boot?  
>>
>> Yes, "mmc part" to be precise.
>>
>>> Because that is the interesting data point, somehow this makes the
>>> eMMC module drive the pin.
>>>   
>>>> When I then unplug the eMMC module, SPI flash works again  
>>>
>>> Again, at runtime?  
>>
>> Yup, all of it at runtime. I felt darey and ignored the fact that you
>> should really only plug the eMMC module at boot time ;).
>>
>> The last step definitely did not change pinmux. Hence the conclusion
>> that this is line drive collision, not pinmux.
>>
>> So I guess you're saying that forcing the eMMC into 4-DAT mode would
>> make things work?
> 
> Technically DS is for HS-400, which is a step beyond 8-bit bus width.
> So just enabling 8 bit should not affect the DS line. There is a bit in
> the MMC controller to enable HS-400, but I don't see that we would set
> it (we actually don't even define the register).
> You could try to force eMMC to 4 bit to check this theory, by
> hack-removing the MMC_MODE_8BIT line in sunxi_mmc_init() in
> drivers/mmc/sunxi-mmc.c (we don't use DM_MMC yet, so it's in the hacky
> part and ignores the DT property)

Ok, that did not help. I guess the next best useful thing to do now
would be to measure what the MISO line shows in each situation.
Unfortunately I don't have my oscilloscope handy :).


Alex
Andre Przywara Jan. 24, 2019, 12:21 p.m. UTC | #12
On Thu, 24 Jan 2019 13:07:54 +0100
Alexander Graf <agraf@suse.de> wrote:

> On 24.01.19 12:31, Andre Przywara wrote:
> > On Thu, 24 Jan 2019 12:21:17 +0100
> > Alexander Graf <agraf@suse.de> wrote:
> >   
> >> On 24.01.19 12:07, Andre Przywara wrote:  
> >>> On Thu, 24 Jan 2019 11:59:32 +0100
> >>> Alexander Graf <agraf@suse.de> wrote:
> >>>     
> >>>> On 24.01.19 11:57, Andre Przywara wrote:    
> >>>>> On Thu, 24 Jan 2019 11:17:03 +0100
> >>>>> Alexander Graf <agraf@suse.de> wrote:
> >>>>>       
> >>>>>> On 22.01.19 17:37, Alexander Graf wrote:      
> >>>>>>>
> >>>>>>>
> >>>>>>> On 22.01.19 17:28, Alexander Graf wrote:        
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 22.01.19 17:17, Oskari Lemmelä wrote:        
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> On 22.1.2019 16.54, Alexander Graf wrote:        
> >>>>>>>>>>
> >>>>>>>>>> On 05.01.19 18:52, Oskari Lemmela wrote:        
> >>>>>>>>>>> Minimal changes to support sun6i based Allwinner SOCs
> >>>>>>>>>>> Changes are based to SPL driver
> >>>>>>>>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
> >>>>>>>>>>>
> >>>>>>>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>        
> >>>>>>>>>> I just tried to see if this patch gives me "sf" access on a
> >>>>>>>>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
> >>>>>>>>>> driver doesn't even get probed?
> >>>>>>>>>>
> >>>>>>>>>> How did you manage to actually make use of this driver?
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Alex        
> >>>>>>>>>
> >>>>>>>>> You need to add spi0 alias to dts and enable needed drivers
> >>>>>>>>> to defconfig. You can take look from Jagan's patch [1].
> >>>>>>>>>
> >>>>>>>>> I didn't include those changes as Maxime already commented
> >>>>>>>>> [2] that Kconfig depends/defaults should be modified
> >>>>>>>>> first.        
> >>>>>>>>
> >>>>>>>> I don't see any relation between the dts change and the
> >>>>>>>> Kconfig dependency issues?
> >>>>>>>>
> >>>>>>>> But thanks for the pointer, I'll give this patch a
> >>>>>>>> try.        
> >>>>>>>
> >>>>>>> Ok, so I now get the SPI controller initialized, but it seems
> >>>>>>> to only returns zeros when trying to access the SPI flash
> >>>>>>> device.
> >>>>>>>
> >>>>>>> In other words, it doesn't work for me :). But maybe I'm
> >>>>>>> missing all the other clk patches to actually get it
> >>>>>>> working?        
> >>>>>>
> >>>>>> Ok, so turns out the problem is that you can't have eMMC and
> >>>>>> SPI both working. I don't quite know yet whether this is just a
> >>>>>> pinmux problem (so potentially software workaroundable) or if
> >>>>>> it's a hardware limitation (eMMC pulling a line required for
> >>>>>> SPI).      
> >>>>>
> >>>>> The line that is shared is "DS", which is only needed for higher
> >>>>> (e)MMC modes (HS-400). At the moment neither Linux nor U-Boot
> >>>>> implement this mode, and probably U-Boot will never need it: it
> >>>>> adds software complexity for a questionable performance benefit,
> >>>>> especially if the flash chips can't keep up with the high
> >>>>> interface speed anyway.
> >>>>>
> >>>>> We addressed this in the DTs (kernel commit fa59dd2ef755).
> >>>>>
> >>>>> I am a bit puzzled here as why this happens: I can't find where
> >>>>> U-Boot would configure PC1 to use the MMC2 function: the
> >>>>> hard-coded mmc_pinmux_setup() doesn't touch PC1 at all. Maybe
> >>>>> it's the BootROM configuring this line? So we would need to
> >>>>> reset it?
> >>>>>
> >>>>> As I don't have an eMMC module here at hand right now, this has
> >>>>> to wait till tonight for further insights.      
> >>>>
> >>>> A few more data points:
> >>>>
> >>>> When I boot w/o eMMC module attached, SPI flash works
> >>>> When I then plug in the eMMC module, SPI flash still works    
> >>>
> >>> Plug in with power off (which you should always do)? So it goes
> >>> through the BootROM?
> >>>     
> >>>> When I that start initializing the eMMC module, SPI flash
> >>>> fails    
> >>>
> >>> What do you mean by that, exactly? Accessing eMMC from U-Boot?    
> >>
> >> Yes, "mmc part" to be precise.
> >>  
> >>> Because that is the interesting data point, somehow this makes the
> >>> eMMC module drive the pin.
> >>>     
> >>>> When I then unplug the eMMC module, SPI flash works again    
> >>>
> >>> Again, at runtime?    
> >>
> >> Yup, all of it at runtime. I felt darey and ignored the fact that
> >> you should really only plug the eMMC module at boot time ;).
> >>
> >> The last step definitely did not change pinmux. Hence the
> >> conclusion that this is line drive collision, not pinmux.
> >>
> >> So I guess you're saying that forcing the eMMC into 4-DAT mode
> >> would make things work?  
> > 
> > Technically DS is for HS-400, which is a step beyond 8-bit bus
> > width. So just enabling 8 bit should not affect the DS line. There
> > is a bit in the MMC controller to enable HS-400, but I don't see
> > that we would set it (we actually don't even define the register).
> > You could try to force eMMC to 4 bit to check this theory, by
> > hack-removing the MMC_MODE_8BIT line in sunxi_mmc_init() in
> > drivers/mmc/sunxi-mmc.c (we don't use DM_MMC yet, so it's in the
> > hacky part and ignores the DT property)  
> 
> Ok, that did not help. I guess the next best useful thing to do now
> would be to measure what the MISO line shows in each situation.
> Unfortunately I don't have my oscilloscope handy :).

Can't you configure PC1 as a gpio, and read that? Not sure the "gpio"
command works with PC1 being a "function pin", but you could always hack
it to GPIO-in by clearing bits[7:4] in 0x01C20848, with md.l/mw.l.

Cheers,
Andre.
Oskari Lemmelä Jan. 24, 2019, 4:09 p.m. UTC | #13
Hi,

On 24.1.2019 12.57, Andre Przywara wrote:
> On Thu, 24 Jan 2019 11:17:03 +0100
> Alexander Graf <agraf@suse.de> wrote:
>
>> On 22.01.19 17:37, Alexander Graf wrote:
>>>
>>> On 22.01.19 17:28, Alexander Graf wrote:  
>>>>
>>>> On 22.01.19 17:17, Oskari Lemmelä wrote:  
>>>>> Hi,
>>>>>
>>>>> On 22.1.2019 16.54, Alexander Graf wrote:  
>>>>>> On 05.01.19 18:52, Oskari Lemmela wrote:  
>>>>>>> Minimal changes to support sun6i based Allwinner SOCs
>>>>>>> Changes are based to SPL driver
>>>>>>> arch/arm/mach-sunxi/spl_spi_sunxi.c
>>>>>>>
>>>>>>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>  
>>>>>> I just tried to see if this patch gives me "sf" access on a
>>>>>> SoPine system. Unfortunately, it seems as if the sun4i_spi
>>>>>> driver doesn't even get probed?
>>>>>>
>>>>>> How did you manage to actually make use of this driver?
>>>>>>
>>>>>>
>>>>>> Alex  
>>>>> You need to add spi0 alias to dts and enable needed drivers to
>>>>> defconfig. You can take look from Jagan's patch [1].
>>>>>
>>>>> I didn't include those changes as Maxime already commented [2]
>>>>> that Kconfig depends/defaults should be modified first.  
>>>> I don't see any relation between the dts change and the Kconfig
>>>> dependency issues?
>>>>
>>>> But thanks for the pointer, I'll give this patch a try.  
>>> Ok, so I now get the SPI controller initialized, but it seems to
>>> only returns zeros when trying to access the SPI flash device.
>>>
>>> In other words, it doesn't work for me :). But maybe I'm missing
>>> all the other clk patches to actually get it working?  
>> Ok, so turns out the problem is that you can't have eMMC and SPI both
>> working. I don't quite know yet whether this is just a pinmux problem
>> (so potentially software workaroundable) or if it's a hardware
>> limitation (eMMC pulling a line required for SPI).
> The line that is shared is "DS", which is only needed for higher (e)MMC
> modes (HS-400). At the moment neither Linux nor U-Boot implement this
> mode, and probably U-Boot will never need it: it adds software
> complexity for a questionable performance benefit, especially if the
> flash chips can't keep up with the high interface speed anyway.
>
> We addressed this in the DTs (kernel commit fa59dd2ef755).
>
> I am a bit puzzled here as why this happens: I can't find where U-Boot
> would configure PC1 to use the MMC2 function: the hard-coded
> mmc_pinmux_setup() doesn't touch PC1 at all. Maybe it's the BootROM
> configuring this line? So we would need to reset it?
>
> As I don't have an eMMC module here at hand right now, this has to wait
> till tonight for further insights.
>
>> But either way, without the eMMC module, things seem to work:
>>
>>   Tested-by: Alexander Graf <agraf@suse.de>
> Does anyone care to clean up the Kconfig dependency mess, since we seem
> to know all the options we need now? Ideally we would just need one or
> two symbols in the *_defconfig files, the rest being automatically
> selected by Kconfig magic.
> It's on my list, but there are other things higher on that one.
I can try contribute and take a look this one.
> Cheers,
> Andre.

Thanks,
Oskari
Alexander Graf Jan. 24, 2019, 9:49 p.m. UTC | #14
On 24.01.19 13:21, Andre Przywara wrote:
> On Thu, 24 Jan 2019 13:07:54 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> On 24.01.19 12:31, Andre Przywara wrote:
>>
>> Ok, that did not help. I guess the next best useful thing to do now
>> would be to measure what the MISO line shows in each situation.
>> Unfortunately I don't have my oscilloscope handy :).
> 
> Can't you configure PC1 as a gpio, and read that? Not sure the "gpio"
> command works with PC1 being a "function pin", but you could always hack
> it to GPIO-in by clearing bits[7:4] in 0x01C20848, with md.l/mw.l.

That showed a 0. But it doesn't really tell any story over time :).

However, I quickly added a bit of debug output to the SD command
emission code. And the spi flash already stops working while the eMMC is
booting up. So this is nothing we configure - the eMMC simply pulls that
line down automatically as soon as it's booted up.

If I configure it to not ever emit CMD1, the SF keeps working, but the
eMMC does not come up unfortunately. I guess it's not compatible to the
pure SD command set.


Alex

---

SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 0 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 8 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 55 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 0 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 1 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 1 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 0 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 1 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 1 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 1 ... allowed
SF: Detected w25q128bv with page size 256 Bytes, erase size 4 KiB, total
16 MiB
MMC CMD 1 ... allowed
SF: unrecognized JEDEC id bytes: 00, 00, 00
Failed to initialize SPI flash at 0:0 (error -2)
MMC CMD 1 ... allowed
SF: unrecognized JEDEC id bytes: 00, 00, 00
Failed to initialize SPI flash at 0:0 (error -2)
MMC CMD 2 ... allowed
SF: unrecognized JEDEC id bytes: 00, 00, 00
Failed to initialize SPI flash at 0:0 (error -2)
MMC CMD 3 ... allowed
[...]


Alex
diff mbox series

Patch

diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
index ee387127f3..4aaa0932d7 100644
--- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
+++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
@@ -321,6 +321,7 @@  struct sunxi_ccm_reg {
 #define AHB_GATE_OFFSET_MMC(n)		(AHB_GATE_OFFSET_MMC0 + (n))
 #define AHB_GATE_OFFSET_DMA		6
 #define AHB_GATE_OFFSET_SS		5
+#define AHB_GATE_OFFSET_SPI0		20
 
 /* ahb_gate1 offsets */
 #define AHB_GATE_OFFSET_DRC0		25
diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun9i.h b/arch/arm/include/asm/arch-sunxi/clock_sun9i.h
index 530e0dd73b..9bbd4d319e 100644
--- a/arch/arm/include/asm/arch-sunxi/clock_sun9i.h
+++ b/arch/arm/include/asm/arch-sunxi/clock_sun9i.h
@@ -194,6 +194,7 @@  struct sunxi_ccm_reg {
 
 /* ahb gate1 field */
 #define AHB_GATE_OFFSET_DMA		24
+#define AHB_GATE_OFFSET_SPI0		20
 
 /* apb1_gate fields */
 #define APB1_GATE_UART_SHIFT		16
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index a7bb5b35c2..88e772cb1a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -219,9 +219,9 @@  config STM32_QSPI
 	  this ST IP core.
 
 config SUN4I_SPI
-	bool "Allwinner A10 SoCs SPI controller"
+	bool "Allwinner SoCs SPI driver"
 	help
-	  SPI driver for Allwinner sun4i, sun5i and sun7i SoCs
+	  SPI driver for Allwinner SoCs
 
 config TEGRA114_SPI
 	bool "nVidia Tegra114 SPI driver"
diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
index 38cc743c61..7af8be15cf 100644
--- a/drivers/spi/sun4i_spi.c
+++ b/drivers/spi/sun4i_spi.c
@@ -37,6 +37,30 @@ 
 
 #define SUN4I_TXDATA_REG	0x04
 
+#ifdef CONFIG_SUNXI_GEN_SUN6I
+#define SUN4I_CTL_REG		0x04
+#define SUN4I_CTL_ENABLE		BIT(0)
+#define SUN4I_CTL_MASTER		BIT(1)
+#define SUN4I_CTL_TP			BIT(7)
+#define SUN4I_CTL_SRST			BIT(31)
+
+#define SUN4I_CTL_CPHA			BIT(0)
+#define SUN4I_CTL_CPOL			BIT(1)
+#define SUN4I_CTL_CS_ACTIVE_LOW		BIT(2)
+#define SUN4I_CTL_CS_MASK		0x30
+#define SUN4I_CTL_CS(cs)		(((cs) << 4) & SUN4I_CTL_CS_MASK)
+#define SUN4I_CTL_CS_MANUAL		BIT(6)
+#define SUN4I_CTL_CS_LEVEL		BIT(7)
+#define SUN4I_CTL_DHB			BIT(8)
+#define SUN4I_CTL_XCH_MASK		0x80000000
+#define SUN4I_CTL_XCH			BIT(31)
+
+#define SUN4I_CTL_RF_RST		BIT(15)
+#define SUN4I_CTL_TF_RST		BIT(31)
+
+#else
+#define SUN4I_CTL_SRST			0
+
 #define SUN4I_CTL_REG		0x08
 #define SUN4I_CTL_ENABLE		BIT(0)
 #define SUN4I_CTL_MASTER		BIT(1)
@@ -54,6 +78,7 @@ 
 #define SUN4I_CTL_CS_MANUAL		BIT(16)
 #define SUN4I_CTL_CS_LEVEL		BIT(17)
 #define SUN4I_CTL_TP			BIT(18)
+#endif
 
 #define SUN4I_INT_CTL_REG	0x0c
 #define SUN4I_INT_CTL_RF_F34		BIT(4)
@@ -92,11 +117,39 @@ 
 #define SUN4I_SPI_DEFAULT_RATE	1000000
 #define SUN4I_SPI_TIMEOUT_US	1000000
 
+#ifdef CONFIG_SUNXI_GEN_SUN6I
+/* sun6i spi register set */
+struct sun4i_spi_regs {
+	u32 res0;
+	u32 ctl;	/* 0x04 */
+	u32 tctl;	/* 0x08 */
+	u32 res1;
+	u32 intctl;	/* 0x10 */
+	u32 st;		/* 0x14 */
+	u32 fifo_ctl;	/* 0x18 */
+	u32 fifo_sta;	/* 0x1c */
+	u32 wait;	/* 0x20 */
+	u32 cctl;	/* 0x24 */
+	u32 res2[2];
+	u32 bc;		/* 0x30 */
+	u32 tc;		/* 0x34 */
+	u32 bctl;	/* 0x38 */
+	u32 res3[113];
+	u32 txdata;	/* 0x200 */
+	u32 res4[63];
+	u32 rxdata;	/* 0x300 */
+};
+#else
 /* sun4i spi register set */
 struct sun4i_spi_regs {
 	u32 rxdata;
 	u32 txdata;
-	u32 ctl;
+	union {
+		u32 ctl;
+		u32 tctl;
+		u32 fifo_ctl;
+		u32 bctl;
+	};
 	u32 intctl;
 	u32 st;
 	u32 dmactl;
@@ -106,6 +159,7 @@  struct sun4i_spi_regs {
 	u32 tc;
 	u32 fifo_sta;
 };
+#endif
 
 struct sun4i_spi_platdata {
 	u32 base_addr;
@@ -149,7 +203,7 @@  static void sun4i_spi_set_cs(struct udevice *bus, u8 cs, bool enable)
 	struct sun4i_spi_priv *priv = dev_get_priv(bus);
 	u32 reg;
 
-	reg = readl(&priv->regs->ctl);
+	reg = readl(&priv->regs->tctl);
 
 	reg &= ~SUN4I_CTL_CS_MASK;
 	reg |= SUN4I_CTL_CS(cs);
@@ -159,7 +213,7 @@  static void sun4i_spi_set_cs(struct udevice *bus, u8 cs, bool enable)
 	else
 		reg |= SUN4I_CTL_CS_LEVEL;
 
-	writel(reg, &priv->regs->ctl);
+	writel(reg, &priv->regs->tctl);
 }
 
 static int sun4i_spi_parse_pins(struct udevice *dev)
@@ -231,7 +285,10 @@  static int sun4i_spi_parse_pins(struct udevice *dev)
 			if (pin < 0)
 				break;
 
-			sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_SPI0);
+			if (IS_ENABLED(CONFIG_MACH_SUN50I))
+				sunxi_gpio_set_cfgpin(pin, SUN50I_GPC_SPI0);
+			else
+				sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_SPI0);
 			sunxi_gpio_set_drv(pin, drive);
 			sunxi_gpio_set_pull(pin, pull);
 		}
@@ -244,10 +301,27 @@  static inline void sun4i_spi_enable_clock(void)
 	struct sunxi_ccm_reg *const ccm =
 		(struct sunxi_ccm_reg *const)SUNXI_CCM_BASE;
 
+#ifdef CONFIG_SUNXI_GEN_SUN6I
+	setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_SPI0));
+#endif
+
 	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_SPI0));
 	writel((1 << 31), &ccm->spi0_clk_cfg);
 }
 
+static inline void sun4i_spi_disable_clock(void)
+{
+	struct sunxi_ccm_reg *const ccm =
+		(struct sunxi_ccm_reg *const)SUNXI_CCM_BASE;
+
+	writel(0, &ccm->spi0_clk_cfg);
+	clrbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_SPI0));
+
+#ifdef CONFIG_SUNXI_GEN_SUN6I
+	clrbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_SPI0));
+#endif
+}
+
 static int sun4i_spi_ofdata_to_platdata(struct udevice *bus)
 {
 	struct sun4i_spi_platdata *plat = dev_get_platdata(bus);
@@ -269,7 +343,6 @@  static int sun4i_spi_probe(struct udevice *bus)
 	struct sun4i_spi_platdata *plat = dev_get_platdata(bus);
 	struct sun4i_spi_priv *priv = dev_get_priv(bus);
 
-	sun4i_spi_enable_clock();
 	sun4i_spi_parse_pins(bus);
 
 	priv->regs = (struct sun4i_spi_regs *)(uintptr_t)plat->base_addr;
@@ -282,9 +355,17 @@  static int sun4i_spi_claim_bus(struct udevice *dev)
 {
 	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
 
+	sun4i_spi_enable_clock();
 	writel(SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER | SUN4I_CTL_TP |
-	       SUN4I_CTL_CS_MANUAL | SUN4I_CTL_CS_ACTIVE_LOW,
+	       SUN4I_CTL_SRST,
 	       &priv->regs->ctl);
+
+	if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
+		while (readl(&priv->regs->ctl) & SUN4I_CTL_SRST)
+			;
+
+	setbits_le32(&priv->regs->tctl, SUN4I_CTL_CS_MANUAL |
+		     SUN4I_CTL_CS_ACTIVE_LOW);
 	return 0;
 }
 
@@ -296,6 +377,7 @@  static int sun4i_spi_release_bus(struct udevice *dev)
 	reg = readl(&priv->regs->ctl);
 	reg &= ~SUN4I_CTL_ENABLE;
 	writel(reg, &priv->regs->ctl);
+	sun4i_spi_disable_clock();
 
 	return 0;
 }
@@ -323,10 +405,10 @@  static int sun4i_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	if (flags & SPI_XFER_BEGIN)
 		sun4i_spi_set_cs(bus, slave_plat->cs, true);
 
-	reg = readl(&priv->regs->ctl);
+	reg = readl(&priv->regs->fifo_ctl);
 
 	/* Reset FIFOs */
-	writel(reg | SUN4I_CTL_RF_RST | SUN4I_CTL_TF_RST, &priv->regs->ctl);
+	writel(reg | SUN4I_CTL_RF_RST | SUN4I_CTL_TF_RST, &priv->regs->fifo_ctl);
 
 	while (len) {
 		/* Setup the transfer now... */
@@ -335,16 +417,18 @@  static int sun4i_spi_xfer(struct udevice *dev, unsigned int bitlen,
 		/* Setup the counters */
 		writel(SUN4I_BURST_CNT(nbytes), &priv->regs->bc);
 		writel(SUN4I_XMIT_CNT(nbytes), &priv->regs->tc);
+		if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
+			writel(SUN4I_BURST_CNT(nbytes), &priv->regs->bctl);
 
 		/* Fill the TX FIFO */
 		sun4i_spi_fill_fifo(priv, nbytes);
 
 		/* Start the transfer */
-		reg = readl(&priv->regs->ctl);
-		writel(reg | SUN4I_CTL_XCH, &priv->regs->ctl);
+		reg = readl(&priv->regs->tctl);
+		writel(reg | SUN4I_CTL_XCH, &priv->regs->tctl);
 
 		/* Wait transfer to complete */
-		ret = wait_for_bit_le32(&priv->regs->ctl, SUN4I_CTL_XCH_MASK,
+		ret = wait_for_bit_le32(&priv->regs->tctl, SUN4I_CTL_XCH_MASK,
 					false, SUN4I_SPI_TIMEOUT_US, false);
 		if (ret) {
 			printf("ERROR: sun4i_spi: Timeout transferring data\n");
@@ -417,7 +501,7 @@  static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
 	struct sun4i_spi_priv *priv = dev_get_priv(dev);
 	u32 reg;
 
-	reg = readl(&priv->regs->ctl);
+	reg = readl(&priv->regs->tctl);
 	reg &= ~(SUN4I_CTL_CPOL | SUN4I_CTL_CPHA);
 
 	if (mode & SPI_CPOL)
@@ -427,7 +511,7 @@  static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
 		reg |= SUN4I_CTL_CPHA;
 
 	priv->mode = mode;
-	writel(reg, &priv->regs->ctl);
+	writel(reg, &priv->regs->tctl);
 
 	return 0;
 }
@@ -441,7 +525,13 @@  static const struct dm_spi_ops sun4i_spi_ops = {
 };
 
 static const struct udevice_id sun4i_spi_ids[] = {
+#ifndef CONFIG_SUNXI_GEN_SUN6I
 	{ .compatible = "allwinner,sun4i-a10-spi"  },
+#else
+	{ .compatible = "allwinner,sun6i-a31-spi" },
+	{ .compatible = "allwinner,sun8i-h3-spi" },
+	{ .compatible = "allwinner,sun50i-a64-spi" },
+#endif
 	{ }
 };