linux-kernel - Re: [PATCH net] macmace: Set platform device coherent_dma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdU1XBqt7hwEW6JTas64ZNGCGCMr5HMZwuLo0O-ZBCOWyA@mail.gmail.com>
Date:   Thu, 3 May 2018 10:46:56 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Finn Thain <fthain@...egraphics.com.au>
Cc:     "David S. Miller" <davem@...emloft.net>,
        linux-m68k <linux-m68k@...ts.linux-m68k.org>,
        netdev <netdev@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH net] macmace: Set platform device coherent_dma_mask

Hi Finn,

CC Christoph

On Thu, May 3, 2018 at 10:38 AM, Finn Thain <fthain@...egraphics.com.au> wrote:
> On Thu, 3 May 2018, Geert Uytterhoeven wrote:
>> > --- a/drivers/net/ethernet/apple/macmace.c
>> > +++ b/drivers/net/ethernet/apple/macmace.c
>> > @@ -203,6 +203,10 @@ static int mace_probe(struct platform_device *pdev)
>> >         unsigned char checksum = 0;
>> >         int err;
>> >
>> > +       err = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> > +       if (err)
>> > +               return err;
>> > +
>> >         dev = alloc_etherdev(PRIV_BYTES);
>> >         if (!dev)
>> >                 return -ENOMEM;
>>
>> Shouldn't this be handled in the platform code that instantiates the
>> device, i.e. in arch/m68k/mac/config.c:mac_platform_init()?
>
> I wondered about that too. The downside is that I'd have to convert
> platform_device_register_simple() into platform_device_register() and add
> all of the boilerplate that goes with that, for little gain.
>
>> Cfr. commit f61e64310b75733d ("m68k: set dma and coherent masks for
>> platform FEC ethernets").
>
> Yes, I looked at that patch before I sent this one. It makes sense to set
> the mask when defining the device since some devices tend to have inherent
> limitations (but that's not really applicable here).
>
> Moreover, it turns out that a number of platform drivers already call
> dma_set_mask_and_coherent() or dma_coerce_mask_and_coherent() or similar.
>
> I figured that platform drivers aren't expected to be particularly
> portable. Well, I'd expect macmace and macsonic to be portable to NuBus
> PowerMacs, but AFAIK the correct mask would remain DMA_BIT_MASK(32).
>
> So that's how I ended up with this patch. But if you are not pursuaded by
> my reasoning then just say the word and I'll take another approach.

Perhaps you can add a new helper (platform_device_register_simple_dma()?)
that takes the DMA mask, too?
With people setting the mask to kill the WARNING splat, this may become
more common.

struct platform_device_info already has a dma_mask field, but
platform_device_register_resndata() explicitly sets it to zero.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds