[U-Boot] [RFC] Tigon III (bcm57xx) Driver on fsl platforms
Ben Collins
ben.c at servergy.com
Thu Dec 19 22:07:54 CET 2013
To answer most of your questions, I'm not entirely worried about the issues you are, I am just looking for help getting it working right now. I'll cross the license/submit bridge when I get to it.
> On Dec 19, 2013, at 2:52 PM, Wolfgang Denk <wd at denx.de> wrote:
>
> Dear Ben Collins,
>
> In message <293C5061-60B5-458B-B586-D66B0ED36983 at servergy.com> you wrote:
>>
>> (Note, I am not subscribed to the list, so please make sure to keep me
>> in Cc)
>
> No, please subscribe if you plan to submit patches.
>
>> I am working on a p4080 based system (Freescale 32-bit Power). In order
>> to support specific peripherals, I needed to have the tg3 driver. The
>> old bcm570x driver that was removed from U-Boot some time ago does not
>> support the 5720 chip that I'm working with (not to mention, the code is
>> horrid), so I took the larger step of porting the FreeBSD if_bge driver
>> to U-Boot.
>
> May I ask why you did not rather use corresponding the Linux driver?
>
>> drivers/net/Makefile | 1 +
>> drivers/net/if_bge.c | 3884
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/net/if_bgereg.h | 3010 ++++++++++++++++++++++++++++++++++++
>
> Urgh. Are you really, really sure all of this is needed?
>
> ...
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + * notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + * notice, this list of conditions and the following disclaimer in the
>> + * documentation and/or other materials provided with the distribution.
>> + * 3. All advertising materials mentioning features or use of this software
>> + * must display the following acknowledgement:
>> + * This product includes software developed by Bill Paul.
>> + * 4. Neither the name of the author nor the names of any co-contributors
>> + * may be used to endorse or promote products derived from this software
>> + * without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY Bill Paul AND CONTRIBUTORS ``AS IS'' AND
>> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>> + * ARE DISCLAIMED. IN NO EVENT SHALL Bill Paul OR THE VOICES IN HIS HEAD
>> + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>> + * THE POSSIBILITY OF SUCH DAMAGE.
>> + */
>
> This is the original aka “4-clause" BSD license with the obnoxious BSD
> advertising clause. It is not compatible with the GNU GPL, so we
> cannot accept this code. Please check if it can be relicensed (which
> may be difficult as you will probably need permission from the
> original author).
>
> Please also note that we do not include full license headers in
> U-Boot; we use SPDX license IDs instead.
>
> In addition, checkpatch reports 679 errors, 311 warnings for your file
> that would need to be cleaned up before you submit it again.
>
> ...
> ...
>> +static void
>> +bge_writemem_direct(struct bge_softc *sc, int off, int val)
>> +{
>> + CSR_WRITE_4(sc, off, val);
>> +}
> ...
>> +/*
>> + * BAR0 MAC register access macros. The Tigon always uses memory mapped
>> register
>> + * accesses and all registers must be accessed with 32 bit operations.
>> + */
>> +#if 1
>> +#define CSR_WRITE_4(sc, reg, val) \
>> + ({ bus_write_4(sc->bge_res, reg, val); udelay(10); })
>> +#define CSR_READ_4(sc, reg) \
>> + ({ u32 val = bus_read_4(sc->bge_res, reg); udelay(10); val; })
>
> This is totally unacceptable. Please make sure to use proper I/O
> accessors instead (which include the required memory barriers so your
> delays will not be needed).
>
>
> Sorry, but this code cannot go into mainline in it's current form.
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> You got to learn three things. What's real, what's not real, and
> what's the difference." - Terry Pratchett, _Witches Abroad_
More information about the U-Boot
mailing list