Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1573489

Re: [PATCH v2 1/2] net: ethernet: bgmac: init sequence bug

From Jon Mason <jon.mason@broadcom.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2 1/2] net: ethernet: bgmac: init sequence bug
Date 2017-02-03 23:40 +0100
Message-ID <t6UGm-809-15@gated-at.bofh.it> (permalink)
References <t6Thf-7ai-3@gated-at.bofh.it> <t6Thf-7ai-1@gated-at.bofh.it> <t6UGm-809-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, Feb 3, 2017 at 4:41 PM, Rafał Miłecki <rafal@milecki.pl> wrote:
> On 02/03/2017 10:08 PM, Jon Mason wrote:
>>
>> @@ -61,15 +60,20 @@ static bool platform_bgmac_clk_enabled(struct bgmac
>> *bgmac)
>>
>>  static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags)
>>  {
>> -       bgmac_idm_write(bgmac, BCMA_IOCTL,
>> -                       (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags));
>> +       u32 val;
>> +
>> +       val = bgmac_idm_read(bgmac, BCMA_IOCTL);
>> +       /* Some bits of BCMA_IOCTL set by HW/ATF and should not change */
>> +       val |= flags & ~(BGMAC_AWCACHE | BGMAC_ARCACHE | BGMAC_AWUSER |
>> +                        BGMAC_ARUSER);
>> +       val |= BGMAC_CLK_EN;
>>         bgmac_idm_read(bgmac, BCMA_IOCTL);
>
>
> This read was previously following write op most likely to flush it or
> something. I don't think it makes any sense to read after read.

Actually, that is sloppy coding on my part.  It should have a write
prior to the read to match what was there before.

I find it odd that it worked when I tested this patch.  It makes me
wonder if this "modify, reset, modify" series is really necessary
after all.  The docs indicate that writing a 0 to the reset brings it
out of reset.  I do not see any code that puts the HW in reset.  So,
unless the bootloader puts the HW in reset or it is in the reset state
by default, this seems like unnecessary code.  I can add some CYA
logic to read and see if it is in reset, toggle the bit, and then just
do the CLK enable.  Thoughts?

Thanks,
Jon

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2 1/2] net: ethernet: bgmac: init sequence bug Jon Mason <jon.mason@broadcom.com> - 2017-02-03 22:10 +0100
  Re: [PATCH v2 1/2] net: ethernet: bgmac: init sequence bug Jon Mason <jon.mason@broadcom.com> - 2017-02-03 23:40 +0100
  Re: [PATCH v2 1/2] net: ethernet: bgmac: init sequence bug Rafał Miłecki <rafal@milecki.pl> - 2017-02-04 00:10 +0100

csiph-web