Re: [PATCH v4] net: Add MOXA ART SoCs ethernet driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/8/2 Mark Rutland <mark.rutland@xxxxxxx>:
> On Thu, Aug 01, 2013 at 10:39:28AM +0100, Jonas Jensen wrote:
>> The MOXA UC-711X hardware(s) has an ethernet controller that seem to be
>> developed internally. The IC used is "RTL8201CP".
>>
>> Since there is no public documentation, this driver is mostly the one
>> published by MOXA that has been heavily cleaned up / ported from linux 2.6.9.
>>
>> Signed-off-by: Jonas Jensen <jonas.jensen@xxxxxxxxx>
>> ---
>
> [...]
>
>> diff --git a/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt b/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
>> new file mode 100644
>> index 0000000..1fc44ff
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
>> @@ -0,0 +1,25 @@
>> +MOXA ART Ethernet Controller
>> +
>> +Required properties:
>> +
>> +- compatible : Must be "moxa,moxart-mac"
>> +- reg : Should contain registers location and length
>> +  index 0 : main register
>
> I assume there's more than one register in the main register bank?
>
> Are there any other register banks not used by this driver?
>
>> +  index 1 : mac address (stored on flash)
>
> Is that flash a part of the MAC unit?
>
> Is it only 6 bytes long (judging by the examples), or is the th only
> portion used by the driver?
>
> If it's bigger, I'd prefer to describe the whole of flash. This driver
> will know the offset within it, and can choose to map only the protion
> it wants to use. Another driver may choose to do more interesting
> things. We need to describe the hardware, not how we expect it to be
> used...
>
> If it's not associated with the MAC, I'm not sure this is the best way
> of describing the linkage...

I also do not quite like it, this is very unusual. Here is how you
could potentially solve this:

- generate a random ethernet MAC adddress and later on let user-space
fetch the MAC address from the flash and set it correctly
- have some early code in arch/arm/mach-moxart which fetches the MAC
address from the flash and then update the ethernet node
local-mac-address property accordingly

Ideally, fetching the MAC address from whatever backend storage is
something that should really be left to user-space...

>
>> +- interrupts : Should contain the mac interrupt number
>
> Is there no need to link this to a phy, or is it a fixed-link device?
>
>> +
>> +Example:
>> +
>> +       mac0: mac@90900000 {
>> +               compatible = "moxa,moxart-mac";
>> +               reg =   <0x90900000 0x100>,
>> +                       <0x80000050 0x6>;
>> +               interrupts = <25 0>;
>> +       };
>> +
>> +       mac1: mac@92000000 {
>> +               compatible = "moxa,moxart-mac";
>> +               reg =   <0x92000000 0x100>,
>> +                       <0x80000056 0x6>;
>> +               interrupts = <27 0>;
>> +       };
>
> [...]
>
>> diff --git a/drivers/net/ethernet/moxa/moxart_ether.h b/drivers/net/ethernet/moxa/moxart_ether.h
>> new file mode 100644
>> index 0000000..790b6a9
>> --- /dev/null
>> +++ b/drivers/net/ethernet/moxa/moxart_ether.h
>> @@ -0,0 +1,513 @@
>> +/* MOXA ART Ethernet (RTL8201CP) driver.
>> + *
>> + * Copyright (C) 2013 Jonas Jensen
>> + *
>> + * Jonas Jensen <jonas.jensen@xxxxxxxxx>
>> + *
>> + * Based on code from
>> + * Moxa Technology Co., Ltd. <www.moxa.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2.  This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#ifndef _MOXART_ETHERNET_H
>> +#define _MOXART_ETHERNET_H
>> +
>> +#define TX_DESC_NUM            64
>> +#define TX_DESC_NUM_MASK       (TX_DESC_NUM-1)
>> +#define TX_NEXT(N)             (((N) + 1) & (TX_DESC_NUM_MASK))
>> +#define TX_BUF_SIZE            1600
>> +
>> +#define RX_DESC_NUM            64
>> +#define RX_DESC_NUM_MASK       (RX_DESC_NUM-1)
>> +#define RX_NEXT(N)             (((N) + 1) & (RX_DESC_NUM_MASK))
>> +#define RX_BUF_SIZE            1600
>> +
>> +struct tx_desc_t {
>> +       union {
>> +#define TXDMA_OWN              BIT(31)
>> +#define TXPKT_EXSCOL           BIT(1)
>> +#define TXPKT_LATECOL          BIT(0)
>> +               unsigned int ui;
>> +
>> +               struct {
>> +                       /* is aborted due to late collision */
>> +                       unsigned int tx_pkt_late_col:1;
>> +
>> +                       /* is aborted after 16 collisions */
>> +                       unsigned int rx_pkt_exs_col:1;
>> +
>> +                       unsigned int reserved1:29;
>> +
>> +                       /* is owned by the MAC controller */
>> +                       unsigned int tx_dma_own:1;
>> +               } ubit;
>> +       } txdes0;
>
> Unless I've misunderstood the later code, this is the format of data
> shared with the device? Bitfields aren't portable, and you can't rely on
> the padding here being what you expect...
>
> Similarly, the other structures below used in this way below seem scary
> to me.
>
>> +
>> +       union {
>> +#define EDOTR                  BIT(31)
>> +#define TXIC                   BIT(30)
>> +#define TX2FIC                 BIT(29)
>> +#define FTS                    BIT(28)
>> +#define LTS                    BIT(27)
>> +#define TXBUF_SIZE_MASK                0x7ff
>> +#define TXBUF_SIZE_MAX         (TXBUF_SIZE_MASK+1)
>> +               unsigned int ui;
>> +
>> +               struct {
>> +                       /* transmit buffer size in byte */
>> +                       unsigned int tx_buf_size:11;
>> +
>> +                       unsigned int reserved2:16;
>> +
>> +                       /* is the last descriptor of a Tx packet */
>> +                       unsigned int lts:1;
>> +
>> +                       /* is the first descriptor of a Tx packet */
>> +                       unsigned int fts:1;
>> +
>> +                       /* transmit to FIFO interrupt on completion */
>> +                       unsigned int tx2_fic:1;
>> +
>> +                       /* transmit interrupt on completion */
>> +                       unsigned int tx_ic:1;
>> +
>> +                       /* end descriptor of transmit ring */
>> +                       unsigned int edotr:1;
>> +               } ubit;
>> +       } txdes1;
>> +
>> +       struct {
>> +               /* transmit buffer physical base address */
>> +               unsigned int addr_phys;
>> +
>> +               /* transmit buffer virtual base address */
>> +               unsigned char *addr_virt;
>> +       } txdes2;
>> +};
>> +
>> +struct rx_desc_t {
>> +       union {
>> +#define RXDMA_OWN              BIT(31)
>> +#define FRS                    BIT(29)
>> +#define LRS                    BIT(28)
>> +#define RX_ODD_NB              BIT(22)
>> +#define RUNT                   BIT(21)
>> +#define FTL                    BIT(20)
>> +#define CRC_ERR                        BIT(19)
>> +#define RX_ERR                 BIT(18)
>> +#define BROADCAST_RXDES0       BIT(17)
>> +#define MULTICAST_RXDES0       BIT(16)
>> +#define RFL_MASK               0x7ff
>> +#define RFL_MAX                        (RFL_MASK+1)
>> +               unsigned int ui;
>> +
>> +               struct {
>> +                       /* receive frame length */
>> +                       unsigned int recv_frame_len:11;
>> +                       unsigned int reserved1:5;
>> +
>> +                       /* multicast frame */
>> +                       unsigned int multicast:1;
>> +
>> +                       /* broadcast frame */
>> +                       unsigned int broadcast:1;
>> +                       unsigned int rx_err:1;          /* receive error */
>> +                       unsigned int crc_err:1;         /* CRC error */
>> +                       unsigned int ftl:1;             /* frame too long */
>> +
>> +                       /* runt packet, less than 64 bytes */
>> +                       unsigned int runt:1;
>> +
>> +                       /* receive odd nibbles */
>> +                       unsigned int rx_odd_nb:1;
>> +                       unsigned int reserved2:5;
>> +
>> +                       /* last receive segment descriptor */
>> +                       unsigned int lrs:1;
>> +
>> +                       /* first receive segment descriptor */
>> +                       unsigned int frs:1;
>> +
>> +                       unsigned int reserved3:1;
>> +                       unsigned int rx_dma_own:1;      /* RXDMA onwership */
>> +               } ubit;
>> +       } rxdes0;
>> +
>> +       union {
>> +#define EDORR                  BIT(31)
>> +#define RXBUF_SIZE_MASK                0x7ff
>> +#define RXBUF_SIZE_MAX         (RXBUF_SIZE_MASK+1)
>> +               unsigned int ui;
>> +
>> +               struct {
>> +                       /* receive buffer size */
>> +                       unsigned int rx_buf_size:11;
>> +
>> +                       unsigned int reserved4:20;
>> +
>> +                       /* end descriptor of receive ring */
>> +                       unsigned int edorr:1;
>> +               } ubit;
>> +       } rxdes1;
>> +
>> +       struct {
>> +               /* receive buffer physical base address */
>> +               unsigned int addr_phys;
>> +
>> +               /* receive buffer virtual base address */
>> +               unsigned char *addr_virt;
>> +       } rxdes2;
>> +};
>> +
>> +struct mac_control_reg_t {
>> +
>> +/* RXDMA has received packets into RX buffer successfully */
>> +#define RPKT_FINISH            BIT(0)
>> +/* receive buffer unavailable */
>> +#define NORXBUF                        BIT(1)
>> +/* TXDMA has moved data into the TX FIFO */
>> +#define XPKT_FINISH            BIT(2)
>> +/* transmit buffer unavailable */
>> +#define NOTXBUF                        BIT(3)
>> +/* packets transmitted to ethernet successfully */
>> +#define XPKT_OK_INT_STS                BIT(4)
>> +/* packets transmitted to ethernet lost due to late
>> + * collision or excessive collision
>> + */
>> +#define XPKT_LOST_INT_STS      BIT(5)
>> +/* packets received into RX FIFO successfully */
>> +#define RPKT_SAV               BIT(6)
>> +/* received packet lost due to RX FIFO full */
>> +#define RPKT_LOST_INT_STS      BIT(7)
>> +#define AHB_ERR                        BIT(8)          /* AHB error */
>> +#define PHYSTS_CHG             BIT(9)          /* PHY link status change */
>> +       unsigned int isr;                       /* interrupt status, 0x0 */
>> +
>> +#define RPKT_FINISH_M          BIT(0)          /* interrupt mask of ISR[0] */
>> +#define NORXBUF_M              BIT(1)          /* interrupt mask of ISR[1] */
>> +#define XPKT_FINISH_M          BIT(2)          /* interrupt mask of ISR[2] */
>> +#define NOTXBUF_M              BIT(3)          /* interrupt mask of ISR[3] */
>> +#define XPKT_OK_M              BIT(4)          /* interrupt mask of ISR[4] */
>> +#define XPKT_LOST_M            BIT(5)          /* interrupt mask of ISR[5] */
>> +#define RPKT_SAV_M             BIT(6)          /* interrupt mask of ISR[6] */
>> +#define RPKT_LOST_M            BIT(7)          /* interrupt mask of ISR[7] */
>> +#define AHB_ERR_M              BIT(8)          /* interrupt mask of ISR[8] */
>> +#define PHYSTS_CHG_M           BIT(9)          /* interrupt mask of ISR[9] */
>> +       unsigned int imr;                       /* interrupt mask, 0x4 */
>> +
>> +/* the most significant 2 bytes of MAC address */
>> +#define MAC_MADR_MASK          0xffff
>> +       /* MAC most significant address, 0x8 */
>> +       unsigned int mac_madr;
>> +
>> +       /* MAC least significant address, 0xc */
>> +       unsigned int mac_ldar;
>> +
>> +       /* multicast address hash table 0, 0x10 */
>> +       unsigned int matht0;
>> +
>> +       /* multicast address hash table 1, 0x14 */
>> +       unsigned int matht1;
>> +
>> +       /* transmit poll demand, 0x18 */
>> +       unsigned int txpd;
>> +
>> +       /* receive poll demand, 0x1c */
>> +       unsigned int rxpd;
>> +
>> +       /* transmit ring base address, 0x20 */
>> +       unsigned int txr_badr;
>> +
>> +       /* receive ring base address, 0x24 */
>> +       unsigned int rxr_badr;
>> +
>> +/* defines the period of TX cycle time */
>> +#define TXINT_TIME_SEL         BIT(15)
>> +#define TXINT_THR_MASK         0x7000
>> +#define TXINT_CNT_MASK         0xf00
>> +/* defines the period of RX cycle time */
>> +#define RXINT_TIME_SEL         BIT(7)
>> +#define RXINT_THR_MASK         0x70
>> +#define RXINT_CNT_MASK         0xF
>> +       /* interrupt timer control, 0x28 */
>> +       unsigned int itc;
>> +
>> +/* defines the period of TX poll time */
>> +#define TXPOLL_TIME_SEL                BIT(12)
>> +#define TXPOLL_CNT_MASK                0xf00
>> +#define TXPOLL_CNT_SHIFT_BIT   8
>> +/* defines the period of RX poll time */
>> +#define RXPOLL_TIME_SEL                BIT(4)
>> +#define RXPOLL_CNT_MASK                0xF
>> +#define RXPOLL_CNT_SHIFT_BIT   0
>> +       /* automatic polling timer control, 0x2c */
>> +       unsigned int aptc;
>> +
>> +/* enable RX FIFO threshold arbitration */
>> +#define RX_THR_EN              BIT(9)
>> +#define RXFIFO_HTHR_MASK       0x1c0
>> +#define RXFIFO_LTHR_MASK       0x38
>> +/* use INCR16 burst command in AHB bus */
>> +#define INCR16_EN              BIT(2)
>> +/* use INCR8 burst command in AHB bus */
>> +#define INCR8_EN               BIT(1)
>> +/* use INCR4 burst command in AHB bus */
>> +#define INCR4_EN               BIT(0)
>> +       /* DMA burst length and arbitration control, 0x30 */
>> +       unsigned int dblac;
>> +
>> +       unsigned int reserved1[21];             /* 0x34 - 0x84 */
>> +
>> +#define RX_BROADPKT            BIT(17)         /* receive boradcast packet */
>> +/* receive all multicast packet */
>> +#define RX_MULTIPKT            BIT(16)
>> +#define FULLDUP                        BIT(15)         /* full duplex */
>> +/* append CRC to transmitted packet */
>> +#define CRC_APD                        BIT(14)
>> +/* do not check incoming packet's destination address */
>> +#define RCV_ALL                        BIT(12)
>> +/* store incoming packet even if its length is great than 1518 bytes */
>> +#define RX_FTL                 BIT(11)
>> +/* store incoming packet even if its length is less than 64 bytes */
>> +#define RX_RUNT                        BIT(10)
>> +/* enable storing incoming packet if the packet passes hash table
>> + * address filtering and is a multicast packet
>> + */
>> +#define HT_MULTI_EN            BIT(9)
>> +#define RCV_EN                 BIT(8)          /* receiver enable */
>> +/* enable packet reception when transmitting packet in half duplex mode */
>> +#define ENRX_IN_HALFTX         BIT(6)
>> +#define XMT_EN                 BIT(5)          /* transmitter enable */
>> +/* disable CRC check when receiving packets */
>> +#define CRC_DIS                        BIT(4)
>> +#define LOOP_EN                        BIT(3)          /* internal loop-back */
>> +/* software reset, last 64 AHB bus clocks */
>> +#define SW_RST                 BIT(2)
>> +#define RDMA_EN                        BIT(1)          /* enable receive DMA chan */
>> +#define XDMA_EN                        BIT(0)          /* enable transmit DMA chan */
>> +       unsigned int maccr;                     /* MAC control, 0x88 */
>> +
>> +#define COL_EXCEED             BIT(11)         /* collisions exceeds 16 */
>> +/* transmitter detects late collision */
>> +#define LATE_COL               BIT(10)
>> +/* packet transmitted to ethernet lost due to late collision
>> + * or excessive collision
>> + */
>> +#define XPKT_LOST              BIT(9)
>> +/* packets transmitted to ethernet successfully */
>> +#define XPKT_OK                        BIT(8)
>> +/* receiver detects a runt packet */
>> +#define RUNT_MAC_STS           BIT(7)
>> +/* receiver detects a frame that is too long */
>> +#define FTL_MAC_STS            BIT(6)
>> +#define CRC_ERR_MAC_STS                BIT(5)
>> +/* received packets list due to RX FIFO full */
>> +#define RPKT_LOST              BIT(4)
>> +/* packets received into RX FIFO successfully */
>> +#define RPKT_SAVE              BIT(3)
>> +/* incoming packet dropped due to collision */
>> +#define COL                    BIT(2)
>> +#define MCPU_BROADCAST         BIT(1)
>> +#define MCPU_MULTICAST         BIT(0)
>> +       unsigned int macsr;                     /* MAC status, 0x8c */
>> +
>> +/* initialize a write sequence to PHY by setting this bit to 1
>> + * this bit would be auto cleared after the write operation is finished.
>> + */
>> +#define MIIWR                  BIT(27)
>> +#define MIIRD                  BIT(26)
>> +#define REGAD_MASK             0x3e00000
>> +#define PHYAD_MASK             0x1f0000
>> +#define MIIRDATA_MASK          0xffff
>> +       unsigned int phycr;                     /* PHY control, 0x90 */
>> +
>> +#define MIIWDATA_MASK          0xffff
>> +       unsigned int phywdata;                  /* PHY write data, 0x94 */
>> +
>> +#define PAUSE_TIME_MASK                0xffff0000
>> +#define FC_HIGH_MASK           0xf000
>> +#define FC_LOW_MASK            0xf00
>> +#define RX_PAUSE               BIT(4)          /* receive pause frame */
>> +/* packet transmission is paused due to receive */
>> +#define TXPAUSED               BIT(3)
>> +       /* pause frame */
>> +/* enable flow control threshold mode. */
>> +#define FCTHR_EN               BIT(2)
>> +#define TX_PAUSE               BIT(1)          /* transmit pause frame */
>> +#define FC_EN                  BIT(0)          /* flow control mode enable */
>> +       unsigned int fcr;                       /* flow control, 0x98 */
>> +
>> +#define BK_LOW_MASK            0xf00
>> +#define BKJAM_LEN_MASK         0xf0
>> +#define BK_MODE                        BIT(1)          /* back pressure address mode */
>> +#define BK_EN                  BIT(0)          /* back pressure mode enable */
>> +       unsigned int bpr;                       /* back pressure, 0x9c */
>> +
>> +       unsigned int reserved2[9];              /* 0xa0 - 0xc0 */
>> +
>> +#define TEST_SEED_MASK      0x3fff
>> +       unsigned int ts;                        /* test seed, 0xc4 */
>> +
>> +#define TXD_REQ                        BIT(31)         /* TXDMA request */
>> +#define RXD_REQ                        BIT(30)         /* RXDMA request */
>> +#define DARB_TXGNT             BIT(29)         /* TXDMA grant */
>> +#define DARB_RXGNT             BIT(28)         /* RXDMA grant */
>> +#define TXFIFO_EMPTY           BIT(27)         /* TX FIFO is empty */
>> +#define RXFIFO_EMPTY           BIT(26)         /* RX FIFO is empty */
>> +#define TXDMA2_SM_MASK         0x7000
>> +#define TXDMA1_SM_MASK         0xf00
>> +#define RXDMA2_SM_MASK         0x70
>> +#define RXDMA1_SM_MASK         0xF
>> +       unsigned int dmafifos;                  /* DMA/FIFO state, 0xc8 */
>> +
>> +#define SINGLE_PKT             BIT(26)         /* single packet mode */
>> +/* automatic polling timer test mode */
>> +#define PTIMER_TEST            BIT(25)
>> +#define ITIMER_TEST            BIT(24)         /* interrupt timer test mode */
>> +#define TEST_SEED_SEL          BIT(22)         /* test seed select */
>> +#define SEED_SEL               BIT(21)         /* seed select */
>> +#define TEST_MODE              BIT(20)         /* transmission test mode */
>> +#define TEST_TIME_MASK         0xffc00
>> +#define TEST_EXCEL_MASK                0x3e0
>> +       unsigned int tm;                        /* test mode, 0xcc */
>> +
>> +       unsigned int reserved3;                 /* 0xd0 */
>> +
>> +#define TX_MCOL_MASK           0xffff0000
>> +#define TX_MCOL_SHIFT_BIT      16
>> +#define TX_SCOL_MASK           0xffff
>> +#define TX_SCOL_SHIFT_BIT      0
>> +       /* TX_MCOL and TX_SCOL counter, 0xd4 */
>> +       unsigned int txmcol_xscol;
>> +
>> +#define RPF_MASK               0xffff0000
>> +#define RPF_SHIFT_BIT          16
>> +#define AEP_MASK               0xffff
>> +#define AEP_SHIFT_BIT          0
>> +       unsigned int rpf_aep;                   /* RPF and AEP counter, 0xd8 */
>> +
>> +#define XM_MASK                        0xffff0000
>> +#define XM_SHIFT_BIT           16
>> +#define PG_MASK                        0xffff
>> +#define PG_SHIFT_BIT           0
>> +       unsigned int xm_pg;                     /* XM and PG counter, 0xdc */
>> +
>> +#define RUNT_CNT_MASK          0xffff0000
>> +#define RUNT_CNT_SHIFT_BIT     16
>> +#define TLCC_MASK              0xffff
>> +#define TLCC_SHIFT_BIT         0
>> +       /* RUNT_CNT and TLCC counter, 0xe0 */
>> +       unsigned int runtcnt_tlcc;
>> +
>> +#define CRCER_CNT_MASK         0xffff0000
>> +#define CRCER_CNT_SHIFT_BIT    16
>> +#define FTL_CNT_MASK           0xffff
>> +#define FTL_CNT_SHIFT_BIT      0
>> +       /* CRCER_CNT and FTL_CNT counter, 0xe4 */
>> +       unsigned int crcercnt_ftlcnt;
>> +
>> +#define RLC_MASK               0xffff0000
>> +#define RLC_SHIFT_BIT          16
>> +#define RCC_MASK               0xffff
>> +#define RCC_SHIFT_BIT          0
>> +       unsigned int rlc_rcc;           /* RLC and RCC counter, 0xe8 */
>> +
>> +       unsigned int broc;              /* BROC counter, 0xec */
>> +       unsigned int mulca;             /* MULCA counter, 0xf0 */
>> +       unsigned int rp;                /* RP counter, 0xf4 */
>> +       unsigned int xp;                /* XP counter, 0xf8 */
>> +};
>
> I don't think you can rely on the fields to be the size you expect (int
> is not necessarily 32 bits), and nor can you expect them to have the
> padding you expect (a 64 bit arch could pad ints to 64 bits). I think it
> would be better to just #define the offsets explicitly, which will be
> safe and easy to verify.
>
> Thanks,
> Mark.



-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux