Dear Wolfgang Denk,
Wolfgang Denk wrote:
> Dear Michal Simek,
>
> In message <1314877154-14536-2-git-send-email-mon...@monstr.eu> you wrote:
>> Add axi_ethernet driver for little-endian Microblaze.
>>
>> RX/TX BDs and rxframe buffer are shared among all axi_ethernet MACs.
>> Only one MAC can work in one time.
>>
>> Signed-off-by: Michal Simek <mon...@monstr.eu>
>
>> +/* Mask used to verify certain PHY features (or register contents)
>> + * in the register above:
>> + * 0x1000: 10Mbps full duplex support
>> + * 0x0800: 10Mbps half duplex support
>> + * 0x0008: Auto-negotiation support
>> + */
>
> Incorrect multiline comment style. Please fix globally.
Fixed.
>
>
>> + u32 timeout = 200;
>> + /* Wait till MDIO interface is ready to accept a new transaction. */
>> + while (timeout && (!(in_be32(®s->mdio_mcr)
>> + & XAE_MDIO_MCR_READY_MASK)))
>> + timeout--;
>
> Multiline statement - braches needed.
>
> Also, add some udelay() or similar to have a defined length of the
> timeout. Please fix globally.
Fixed.
>
>
>> +/* STOP DMA transfers */
>> +static void axiemac_halt(struct eth_device *dev)
>> +{
>> + struct axidma_priv *priv = dev->priv;
>> +
>> + /* Stop the hardware */
>> + priv->dmatx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;
>> + priv->dmarx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;
>
> Please ALWAYS use I/O accessors to access device registers. Please
> fix globally.
Done
>
>> +static int IsRxReady(struct eth_device *dev)
>
> As requested before: Please get rid of these CamelCaps identifiers!!!
Fixed this.
>
>
>> +static int axiemac_miiphy_read(const char *devname, uchar addr,
>> + uchar reg, ushort *val)
>> +{
>> + struct eth_device *dev = eth_get_dev();
>> + *val = phyread(dev, addr, reg);
>> + debug("axiemac: Read MII 0x%x, 0x%x, 0x%x\n", addr, reg, *val);
>> + return 0;
>
> Please separate declarations and code by a single blank line. Please
> fix globally.
Done
>
>
> Why does this function return int when you uncondsitionally always
> return 0 only?
I have fixed phywrite/phyread functions.
>
>> +}
>> +
>> +static int axiemac_miiphy_write(const char *devname, uchar addr,
>> + uchar reg, ushort val)
>> +{
>> + struct eth_device *dev = eth_get_dev();
>> + debug("axiemac: Write MII 0x%x, 0x%x, 0x%x\n", addr, reg, val);
>> + phywrite(dev, addr, reg, val);
>> + return 0;
>> +}
>
> Ditto.
>
>> +static int axiemac_bus_reset(struct mii_dev *bus)
>> +{
>> + debug("axiemac: Bus reset\n");
>> + return 0;
>> +}
>
> Ditto.
Because of miiphyutil code requires to return 0 and I am not aware about
doing anything useful for bus_reset.
Regards,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
