[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