dmaengine: ARM pl080 misidentified as Samsung pl080s

dmaengine: ARM pl080 misidentified as Samsung pl080s

[Johannes Stezenbach]

Hi,

commit da1b6c05b8b5bd ("dmaengine: PL08x: Add support for PL080S variant")
added this hunk:

 static struct amba_id pl08x_ids[] = {
+       /* Samsung PL080S variant */
+       {
+               .id     = 0x0a141080,
+               .mask   = 0xffffffff,
+               .data   = &vendor_pl080s,
+       },
        /* PL080 */

However, 0x0a141080 happens to be the ID of the
ARM PL080, too:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0196g/I54627.html

Since the PL080S has the configuration register at a different
address the driver doesn't work for ARM PL080 anymore.


Johannes

dmaengine: ARM pl080 misidentified as Samsung pl080s

[Russell King - ARM Linux]

On Thu, Mar 24, 2016 at 11:56:51AM +0100, Johannes Stezenbach wrote:
> Hi,
> 
> commit da1b6c05b8b5bd ("dmaengine: PL08x: Add support for PL080S variant")
> added this hunk:
> 
>  static struct amba_id pl08x_ids[] = {
> +       /* Samsung PL080S variant */
> +       {
> +               .id     = 0x0a141080,
> +               .mask   = 0xffffffff,
> +               .data   = &vendor_pl080s,
> +       },
>         /* PL080 */
> 
> However, 0x0a141080 happens to be the ID of the
> ARM PL080, too:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0196g/I54627.html
> 
> Since the PL080S has the configuration register at a different
> address the driver doesn't work for ARM PL080 anymore.

Vendors have got into serious trouble in the past over IDs claiming that
some other legal entity being the vendor.  A certain buggy ARM926 clone
comes to mind, and they were forced to respin their silicon as a result.
It sounds like Samsung have made the same idiotic mistake.  I suggest
reverting the offending commit.  This behaviour really isn't on.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

dmaengine: ARM pl080 misidentified as Samsung pl080s

[Tomasz Figa]

2016-03-24 11:56 GMT+01:00 Johannes Stezenbach <js@sig21.net>:
> Hi,
>
> commit da1b6c05b8b5bd ("dmaengine: PL08x: Add support for PL080S variant")
> added this hunk:
>
>  static struct amba_id pl08x_ids[] = {
> +       /* Samsung PL080S variant */
> +       {
> +               .id     = 0x0a141080,
> +               .mask   = 0xffffffff,
> +               .data   = &vendor_pl080s,
> +       },
>         /* PL080 */
>
> However, 0x0a141080 happens to be the ID of the
> ARM PL080, too:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0196g/I54627.html

It can be, assuming that 0xa1 is an existing revision of original
PL080. AFAIK when we were figuring out what to do with this broken
Samsung's controller, we found out that there was no platform with
such revision. Is your concern related to any actual hardware already
supported by upstream?

Best regards,
Tomasz

>
> Since the PL080S has the configuration register at a different
> address the driver doesn't work for ARM PL080 anymore.
>
>
> Johannes

dmaengine: ARM pl080 misidentified as Samsung pl080s

[Johannes Stezenbach]

On Sat, Mar 26, 2016 at 12:08:00AM +0100, Tomasz Figa wrote:
> 2016-03-24 11:56 GMT+01:00 Johannes Stezenbach <js@sig21.net>:
> >
> > commit da1b6c05b8b5bd ("dmaengine: PL08x: Add support for PL080S variant")
> > added this hunk:
> >
> >  static struct amba_id pl08x_ids[] = {
> > +       /* Samsung PL080S variant */
> > +       {
> > +               .id     = 0x0a141080,
> > +               .mask   = 0xffffffff,
> > +               .data   = &vendor_pl080s,
> > +       },
> >         /* PL080 */
> >
> > However, 0x0a141080 happens to be the ID of the
> > ARM PL080, too:
> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0196g/I54627.html
> 
> It can be, assuming that 0xa1 is an existing revision of original
> PL080. AFAIK when we were figuring out what to do with this broken
> Samsung's controller, we found out that there was no platform with
> such revision. Is your concern related to any actual hardware already
> supported by upstream?

It isn't upstream and likely won't.  I reported it because
this is ARM IP probably used by others, for me it was
easy enough to fix locally but I wasted two hours until
I figured out what went wrong.

Johannes