diff mbox series

[net-next,v2,7/9] net: ethernet: oa_tc6: implement data transaction interface

Message ID 20231023154649.45931-8-Parthiban.Veerasooran@microchip.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Add support for OPEN Alliance 10BASE-T1x MACPHY Serial Interface | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1364 this patch: 1364
netdev/cc_maintainers warning 1 maintainers not CCed: parthiban.veerasooran@microchip.com
netdev/build_clang success Errors and warnings before: 1389 this patch: 1389
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1389 this patch: 1389
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 92 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 1 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

Parthiban Veerasooran Oct. 23, 2023, 3:46 p.m. UTC
The ethernet frame to be sent to MAC-PHY is converted into multiple
transmit data chunks. A transmit data chunk consists of a 4-byte data
header followed by the transmit data chunk payload.

The received ethernet frame from the network is converted into multiple
receive data chunks by the MAC-PHY and a receive data chunk consists of
the receive data chunk payload followed by a 4-byte data footer at the
end.

The MAC-PHY shall support a default data chunk payload size of 64 bytes.
Data chunk payload sizes of 32, 16, or 8 bytes may also be supported. The
data chunk payload is always a multiple of 4 bytes.

The 4-byte data header occurs at the beginning of each transmit data
chunk on MOSI and the 4-byte data footer occurs at the end of each
receive data chunk on MISO. The data header and footer contain the
information needed to determine the validity and location of the transmit
and receive frame data within the data chunk payload. Ethernet frames
shall be aligned to a 32-bit boundary within the data chunk payload.

Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
---
 drivers/net/ethernet/oa_tc6.c | 546 +++++++++++++++++++++++++++++++++-
 include/linux/oa_tc6.h        |  47 ++-
 2 files changed, 591 insertions(+), 2 deletions(-)

Comments

Andrew Lunn Oct. 24, 2023, 2:07 a.m. UTC | #1
> +static u16 oa_tc6_prepare_empty_chunk(struct oa_tc6 *tc6, u8 *buf, u8 cp_count)
> +{
> +	u32 hdr;
> +
> +	/* Prepare empty chunks used for getting interrupt information or if
> +	 * receive data available.
> +	 */
> +	for (u8 i = 0; i < cp_count; i++) {
> +		hdr = FIELD_PREP(DATA_HDR_DNC, 1);
> +		hdr |= FIELD_PREP(DATA_HDR_P, oa_tc6_get_parity(hdr));
> +		*(__be32 *)&buf[i * (tc6->cps + TC6_HDR_SIZE)] = cpu_to_be32(hdr);
> +		memset(&buf[TC6_HDR_SIZE + (i * (tc6->cps + TC6_HDR_SIZE))], 0,
> +		       tc6->cps);
> +	}

This is not simple, and its the sort of code which makes me wounder if
its gone off the end of the buffer. It would be good to find somebody
internally within Microchip to review this code.

> +static void oa_tc6_rx_eth_ready(struct oa_tc6 *tc6)
> +{
> +	struct sk_buff *skb;
> +
> +	/* Send the received ethernet packet to network layer */
> +	skb = netdev_alloc_skb(tc6->netdev, tc6->rxd_bytes + NET_IP_ALIGN);
> +	if (!skb) {
> +		tc6->netdev->stats.rx_dropped++;
> +		netdev_dbg(tc6->netdev, "Out of memory for rx'd frame");

You can just return here, and skip the else. Less indentation is
better, it generally makes the code more readable.

> +	} else {
> +		skb_reserve(skb, NET_IP_ALIGN);
> +		memcpy(skb_put(skb, tc6->rxd_bytes), &tc6->eth_rx_buf[0],
> +		       tc6->rxd_bytes);
> +		skb->protocol = eth_type_trans(skb, tc6->netdev);
> +		tc6->netdev->stats.rx_packets++;
> +		tc6->netdev->stats.rx_bytes += tc6->rxd_bytes;
> +		/* 0 for NET_RX_SUCCESS and 1 for NET_RX_DROP */
> +		if (netif_rx(skb))
> +			tc6->netdev->stats.rx_dropped++;

Rather than have a comment do:

		if (netif_rx(skb) == NET_RX_DROP)
			tc6->netdev->stats.rx_dropped++;


> +static void oa_tc6_rx_eth_complete2(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
> +{
> +	u16 ebo;

What does ftr and ebo mean? Its really hard to read this code because
the names are not really meaningful.

> +
> +	if (FIELD_GET(DATA_FTR_EV, ftr))
> +		ebo = FIELD_GET(DATA_FTR_EBO, ftr) + 1;
> +	else
> +		ebo = tc6->cps;
> +
> +	memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[0], ebo);
> +	tc6->rxd_bytes += ebo;
> +	if (FIELD_GET(DATA_FTR_EV, ftr)) {
> +		/* If EV set then send the received ethernet frame to n/w */
> +		oa_tc6_rx_eth_ready(tc6);
> +		tc6->rxd_bytes = 0;
> +		tc6->rx_eth_started = false;
> +	}
> +}
> +
> +static void oa_tc6_rx_eth_complete1(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
> +{
> +	u16 ebo;
> +	u16 sbo;
> +
> +	sbo = FIELD_GET(DATA_FTR_SWO, ftr) * 4;
> +	ebo = FIELD_GET(DATA_FTR_EBO, ftr) + 1;
> +
> +	if (ebo <= sbo) {
> +		memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[0], ebo);
> +		tc6->rxd_bytes += ebo;
> +		oa_tc6_rx_eth_ready(tc6);
> +		tc6->rxd_bytes = 0;
> +		memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo],
> +		       tc6->cps - sbo);
> +		tc6->rxd_bytes += (tc6->cps - sbo);
> +	} else {
> +		memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo],
> +		       ebo - sbo);
> +		tc6->rxd_bytes += (ebo - sbo);
> +		oa_tc6_rx_eth_ready(tc6);
> +		tc6->rxd_bytes = 0;
> +	}
> +}
> +
> +static void oa_tc6_start_rx_eth(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
> +{
> +	u16 sbo;
> +
> +	tc6->rxd_bytes = 0;
> +	tc6->rx_eth_started = true;
> +	sbo = FIELD_GET(DATA_FTR_SWO, ftr) * 4;
> +	memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo], tc6->cps - sbo);
> +	tc6->rxd_bytes += (tc6->cps - sbo);
> +}
> +
> +static u32 oa_tc6_get_footer(struct oa_tc6 *tc6, u8 *buf, u8 cp_num)
> +{
> +	__be32 ftr;
> +
> +	ftr = *(__be32 *)&buf[tc6->cps + (cp_num * (tc6->cps + TC6_FTR_SIZE))];
> +
> +	return be32_to_cpu(ftr);
> +}
> +
> +static void oa_tc6_update_txc_rca(struct oa_tc6 *tc6, u32 ftr)
> +{
> +	tc6->txc = FIELD_GET(DATA_FTR_TXC, ftr);
> +	tc6->rca = FIELD_GET(DATA_FTR_RCA, ftr);
> +}
> +
> +static int oa_tc6_check_ftr_errors(struct oa_tc6 *tc6, u32 ftr)
> +{
> +	/* Check for footer parity error */
> +	if (oa_tc6_get_parity(ftr)) {
> +		net_err_ratelimited("%s: Footer parity error\n",
> +				    tc6->netdev->name);
> +		return FTR_ERR;
> +	}
> +	/* If EXST set in the footer then read STS0 register to get the
> +	 * status information.
> +	 */
> +	if (FIELD_GET(DATA_FTR_EXST, ftr)) {
> +		if (oa_tc6_process_exst(tc6))
> +			net_err_ratelimited("%s: Failed to process EXST\n",
> +					    tc6->netdev->name);
> +		return FTR_ERR;
> +	}
> +	if (FIELD_GET(DATA_FTR_HDRB, ftr)) {
> +		net_err_ratelimited("%s: Footer eeceived header bad\n",
> +				    tc6->netdev->name);
> +		return FTR_ERR;
> +	}
> +	if (!FIELD_GET(DATA_FTR_SYNC, ftr)) {
> +		net_err_ratelimited("%s: Footer configuration unsync\n",
> +				    tc6->netdev->name);
> +		return FTR_ERR;
> +	}
> +	return FTR_OK;
> +}
> +
> +static void oa_tc6_drop_rx_eth(struct oa_tc6 *tc6)
> +{
> +	tc6->rxd_bytes = 0;
> +	tc6->rx_eth_started = false;
> +	tc6->netdev->stats.rx_dropped++;
> +	net_err_ratelimited("%s: Footer frame drop\n",
> +			    tc6->netdev->name);
> +}
> +
> +static int oa_tc6_process_rx_chunks(struct oa_tc6 *tc6, u8 *buf, u16 len)
> +{
> +	u8 cp_count;
> +	u8 *payload;
> +	u32 ftr;
> +	int ret;
> +
> +	/* Calculate the number of chunks received */
> +	cp_count = len / (tc6->cps + TC6_FTR_SIZE);
> +
> +	for (u8 i = 0; i < cp_count; i++) {
> +		/* Get the footer and payload */
> +		ftr = oa_tc6_get_footer(tc6, buf, i);
> +		payload = &buf[(i * (tc6->cps + TC6_FTR_SIZE))];

This would be more readable:

	/* Calculate the number of chunks received */
	chunks = len / (tc6->cps + TC6_FTR_SIZE);

	for (u8 chunk = 0; chunk < chunks; chunk++) {
		/* Get the footer and payload */
		ftr = oa_tc6_get_footer(tc6, buf, chunk);
		payload = &buf[(chunk * (tc6->cps + TC6_FTR_SIZE))];

etc.

And maybe move most of this code into a function
oa_tc6_process_rx_chunk(). With lots of small functions with good
names, you need less comments.


> +		/* Check for data valid */
> +		if (FIELD_GET(DATA_FTR_DV, ftr)) {
> +			/* Check whether both start valid and end valid are in a
> +			 * single chunk payload means a single chunk payload may
> +			 * contain an entire ethernet frame.
> +			 */
> +			if (FIELD_GET(DATA_FTR_SV, ftr) &&
> +			    FIELD_GET(DATA_FTR_EV, ftr)) {


		if (FIELD_GET(DATA_FOOTER_START_VALID, footer) &&
		    FIELD_GET(DATA_FOOTER_END_VALID, footer)) {

Don't you think that is more readable?

> +static void oa_tc6_prepare_tx_chunks(struct oa_tc6 *tc6, u8 *buf,
> +				     struct sk_buff *skb)
> +{
> +	bool frame_started = false;
> +	u16 copied_bytes = 0;
> +	u16 copy_len;
> +	u32 hdr;
> +
> +	/* Calculate the number tx credit counts needed to transport the tx
> +	 * ethernet frame.
> +	 */
> +	tc6->txc_needed = (skb->len / tc6->cps) + ((skb->len % tc6->cps) ? 1 : 0);

Why call it a credit here, but a chunk when receiving?

> +static int oa_tc6_perform_spi_xfer(struct oa_tc6 *tc6)
> +{
> +	bool do_tx_again;
> +	u16 total_len;
> +	u16 rca_len;
> +	u16 tx_len;
> +	int ret;
> +
> +	do {
> +		do_tx_again = false;
> +		rca_len = 0;
> +		tx_len = 0;
> +
> +		/* In case of an interrupt, perform an empty chunk transfer to
> +		 * know the purpose of the interrupt. Interrupt may occur in
> +		 * case of RCA (Receive Chunk Available) and TXC (Transmit
> +		 * Credit Count). Both will occur if they are not indicated
> +		 * through the previous footer.
> +		 */
> +		if (tc6->int_flag) {
> +			tc6->int_flag = false;
> +			total_len = oa_tc6_prepare_empty_chunk(tc6,
> +							       tc6->spi_tx_buf,
> +							       1);
> +		} else {
> +			/* Calculate the transfer length */
> +			if (tc6->tx_flag && tc6->txc) {
> +				tx_len = oa_tc6_calculate_tx_len(tc6);
> +				memcpy(&tc6->spi_tx_buf[0],
> +				       &tc6->eth_tx_buf[tc6->tx_pos], tx_len);
> +			}
> +
> +			if (tc6->rca)
> +				rca_len = oa_tc6_calculate_rca_len(tc6, tx_len);
> +
> +			total_len = tx_len + rca_len;
> +		}
> +		ret = oa_tc6_spi_transfer(tc6->spi, tc6->spi_tx_buf,
> +					  tc6->spi_rx_buf, total_len);
> +		if (ret)
> +			return ret;
> +		/* Process the rxd chunks to get the ethernet frame or status */
> +		ret = oa_tc6_process_rx_chunks(tc6, tc6->spi_rx_buf, total_len);
> +		if (ret)
> +			return ret;
> +		if (tc6->tx_flag) {
> +			tc6->tx_pos += tx_len;
> +			tc6->txc_needed = tc6->txc_needed -
> +					  (tx_len / (tc6->cps + TC6_HDR_SIZE));
> +			/* If the complete ethernet frame is transmitted then
> +			 * return the skb and update the details to n/w layer.
> +			 */
> +			if (!tc6->txc_needed)
> +				oa_tc6_tx_eth_complete(tc6);
> +			else if (tc6->txc)
> +				/* If txc is available again and updated from
> +				 * the previous footer then perform tx again.
> +				 */
> +				do_tx_again = true;
> +		}
> +
> +		/* If rca is updated from the previous footer then perform empty
> +		 * tx to receive ethernet frame.
> +		 */
> +		if (tc6->rca)
> +			do_tx_again = true;
> +	} while (do_tx_again);

The coding standard say:

Functions should be short and sweet, and do just one thing. They
should fit on one or two screenfuls of text (the ISO/ANSI screen size
is 80x24, as we all know), and do one thing and do that well.

This is too long, and does too many things.

     Andrew
Parthiban Veerasooran Oct. 31, 2023, 8:26 a.m. UTC | #2
Hi Andrew,

On 24/10/23 7:37 am, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> +static u16 oa_tc6_prepare_empty_chunk(struct oa_tc6 *tc6, u8 *buf, u8 cp_count)
>> +{
>> +     u32 hdr;
>> +
>> +     /* Prepare empty chunks used for getting interrupt information or if
>> +      * receive data available.
>> +      */
>> +     for (u8 i = 0; i < cp_count; i++) {
>> +             hdr = FIELD_PREP(DATA_HDR_DNC, 1);
>> +             hdr |= FIELD_PREP(DATA_HDR_P, oa_tc6_get_parity(hdr));
>> +             *(__be32 *)&buf[i * (tc6->cps + TC6_HDR_SIZE)] = cpu_to_be32(hdr);
>> +             memset(&buf[TC6_HDR_SIZE + (i * (tc6->cps + TC6_HDR_SIZE))], 0,
>> +                    tc6->cps);
>> +     }
> 
> This is not simple, and its the sort of code which makes me wounder if
> its gone off the end of the buffer. It would be good to find somebody
> internally within Microchip to review this code.
I think, I don't need to do memset here as the header itself doesn't 
describe any valid information about the payload except data not control 
as it is an empty chunk and no matter what the payload contains. Apart 
from that I don't get what's wrong here, anyway I will ask our internal 
reviewers to review the code.
> 
>> +static void oa_tc6_rx_eth_ready(struct oa_tc6 *tc6)
>> +{
>> +     struct sk_buff *skb;
>> +
>> +     /* Send the received ethernet packet to network layer */
>> +     skb = netdev_alloc_skb(tc6->netdev, tc6->rxd_bytes + NET_IP_ALIGN);
>> +     if (!skb) {
>> +             tc6->netdev->stats.rx_dropped++;
>> +             netdev_dbg(tc6->netdev, "Out of memory for rx'd frame");
> 
> You can just return here, and skip the else. Less indentation is
> better, it generally makes the code more readable.
Ah yes. Noted.
> 
>> +     } else {
>> +             skb_reserve(skb, NET_IP_ALIGN);
>> +             memcpy(skb_put(skb, tc6->rxd_bytes), &tc6->eth_rx_buf[0],
>> +                    tc6->rxd_bytes);
>> +             skb->protocol = eth_type_trans(skb, tc6->netdev);
>> +             tc6->netdev->stats.rx_packets++;
>> +             tc6->netdev->stats.rx_bytes += tc6->rxd_bytes;
>> +             /* 0 for NET_RX_SUCCESS and 1 for NET_RX_DROP */
>> +             if (netif_rx(skb))
>> +                     tc6->netdev->stats.rx_dropped++;
> 
> Rather than have a comment do:
Ah ok, will do it.
> 
>                  if (netif_rx(skb) == NET_RX_DROP)
>                          tc6->netdev->stats.rx_dropped++;
> 
> 
>> +static void oa_tc6_rx_eth_complete2(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
>> +{
>> +     u16 ebo;
> 
> What does ftr and ebo mean? Its really hard to read this code because
> the names are not really meaningful.
Ok, ftr -> footer and ebo -> end_byte_offset. Will update in the next 
revision.
> 
>> +
>> +     if (FIELD_GET(DATA_FTR_EV, ftr))
>> +             ebo = FIELD_GET(DATA_FTR_EBO, ftr) + 1;
>> +     else
>> +             ebo = tc6->cps;
>> +
>> +     memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[0], ebo);
>> +     tc6->rxd_bytes += ebo;
>> +     if (FIELD_GET(DATA_FTR_EV, ftr)) {
>> +             /* If EV set then send the received ethernet frame to n/w */
>> +             oa_tc6_rx_eth_ready(tc6);
>> +             tc6->rxd_bytes = 0;
>> +             tc6->rx_eth_started = false;
>> +     }
>> +}
>> +
>> +static void oa_tc6_rx_eth_complete1(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
>> +{
>> +     u16 ebo;
>> +     u16 sbo;
>> +
>> +     sbo = FIELD_GET(DATA_FTR_SWO, ftr) * 4;
>> +     ebo = FIELD_GET(DATA_FTR_EBO, ftr) + 1;
>> +
>> +     if (ebo <= sbo) {
>> +             memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[0], ebo);
>> +             tc6->rxd_bytes += ebo;
>> +             oa_tc6_rx_eth_ready(tc6);
>> +             tc6->rxd_bytes = 0;
>> +             memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo],
>> +                    tc6->cps - sbo);
>> +             tc6->rxd_bytes += (tc6->cps - sbo);
>> +     } else {
>> +             memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo],
>> +                    ebo - sbo);
>> +             tc6->rxd_bytes += (ebo - sbo);
>> +             oa_tc6_rx_eth_ready(tc6);
>> +             tc6->rxd_bytes = 0;
>> +     }
>> +}
>> +
>> +static void oa_tc6_start_rx_eth(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
>> +{
>> +     u16 sbo;
>> +
>> +     tc6->rxd_bytes = 0;
>> +     tc6->rx_eth_started = true;
>> +     sbo = FIELD_GET(DATA_FTR_SWO, ftr) * 4;
>> +     memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo], tc6->cps - sbo);
>> +     tc6->rxd_bytes += (tc6->cps - sbo);
>> +}
>> +
>> +static u32 oa_tc6_get_footer(struct oa_tc6 *tc6, u8 *buf, u8 cp_num)
>> +{
>> +     __be32 ftr;
>> +
>> +     ftr = *(__be32 *)&buf[tc6->cps + (cp_num * (tc6->cps + TC6_FTR_SIZE))];
>> +
>> +     return be32_to_cpu(ftr);
>> +}
>> +
>> +static void oa_tc6_update_txc_rca(struct oa_tc6 *tc6, u32 ftr)
>> +{
>> +     tc6->txc = FIELD_GET(DATA_FTR_TXC, ftr);
>> +     tc6->rca = FIELD_GET(DATA_FTR_RCA, ftr);
>> +}
>> +
>> +static int oa_tc6_check_ftr_errors(struct oa_tc6 *tc6, u32 ftr)
>> +{
>> +     /* Check for footer parity error */
>> +     if (oa_tc6_get_parity(ftr)) {
>> +             net_err_ratelimited("%s: Footer parity error\n",
>> +                                 tc6->netdev->name);
>> +             return FTR_ERR;
>> +     }
>> +     /* If EXST set in the footer then read STS0 register to get the
>> +      * status information.
>> +      */
>> +     if (FIELD_GET(DATA_FTR_EXST, ftr)) {
>> +             if (oa_tc6_process_exst(tc6))
>> +                     net_err_ratelimited("%s: Failed to process EXST\n",
>> +                                         tc6->netdev->name);
>> +             return FTR_ERR;
>> +     }
>> +     if (FIELD_GET(DATA_FTR_HDRB, ftr)) {
>> +             net_err_ratelimited("%s: Footer eeceived header bad\n",
>> +                                 tc6->netdev->name);
>> +             return FTR_ERR;
>> +     }
>> +     if (!FIELD_GET(DATA_FTR_SYNC, ftr)) {
>> +             net_err_ratelimited("%s: Footer configuration unsync\n",
>> +                                 tc6->netdev->name);
>> +             return FTR_ERR;
>> +     }
>> +     return FTR_OK;
>> +}
>> +
>> +static void oa_tc6_drop_rx_eth(struct oa_tc6 *tc6)
>> +{
>> +     tc6->rxd_bytes = 0;
>> +     tc6->rx_eth_started = false;
>> +     tc6->netdev->stats.rx_dropped++;
>> +     net_err_ratelimited("%s: Footer frame drop\n",
>> +                         tc6->netdev->name);
>> +}
>> +
>> +static int oa_tc6_process_rx_chunks(struct oa_tc6 *tc6, u8 *buf, u16 len)
>> +{
>> +     u8 cp_count;
>> +     u8 *payload;
>> +     u32 ftr;
>> +     int ret;
>> +
>> +     /* Calculate the number of chunks received */
>> +     cp_count = len / (tc6->cps + TC6_FTR_SIZE);
>> +
>> +     for (u8 i = 0; i < cp_count; i++) {
>> +             /* Get the footer and payload */
>> +             ftr = oa_tc6_get_footer(tc6, buf, i);
>> +             payload = &buf[(i * (tc6->cps + TC6_FTR_SIZE))];
> 
> This would be more readable:
> 
>          /* Calculate the number of chunks received */
>          chunks = len / (tc6->cps + TC6_FTR_SIZE);
> 
>          for (u8 chunk = 0; chunk < chunks; chunk++) {
>                  /* Get the footer and payload */
>                  ftr = oa_tc6_get_footer(tc6, buf, chunk);
>                  payload = &buf[(chunk * (tc6->cps + TC6_FTR_SIZE))];
> 
> etc.
Ok thanks for the input. Will do it.
> 
> And maybe move most of this code into a function
> oa_tc6_process_rx_chunk(). With lots of small functions with good
> names, you need less comments.
Yes sure. Will do it.
> 
> 
>> +             /* Check for data valid */
>> +             if (FIELD_GET(DATA_FTR_DV, ftr)) {
>> +                     /* Check whether both start valid and end valid are in a
>> +                      * single chunk payload means a single chunk payload may
>> +                      * contain an entire ethernet frame.
>> +                      */
>> +                     if (FIELD_GET(DATA_FTR_SV, ftr) &&
>> +                         FIELD_GET(DATA_FTR_EV, ftr)) {
> 
> 
>                  if (FIELD_GET(DATA_FOOTER_START_VALID, footer) &&
>                      FIELD_GET(DATA_FOOTER_END_VALID, footer)) {
> 
> Don't you think that is more readable?
Yes it is more readable now. Will update in the next revision.
> 
>> +static void oa_tc6_prepare_tx_chunks(struct oa_tc6 *tc6, u8 *buf,
>> +                                  struct sk_buff *skb)
>> +{
>> +     bool frame_started = false;
>> +     u16 copied_bytes = 0;
>> +     u16 copy_len;
>> +     u32 hdr;
>> +
>> +     /* Calculate the number tx credit counts needed to transport the tx
>> +      * ethernet frame.
>> +      */
>> +     tc6->txc_needed = (skb->len / tc6->cps) + ((skb->len % tc6->cps) ? 1 : 0);
> 
> Why call it a credit here, but a chunk when receiving?
I named as the tx path always gives the number of tx credits counts can 
be enqueued for transfer. So used txc needed name to represent. Ok I 
will change it as chunks_needed.
> 
>> +static int oa_tc6_perform_spi_xfer(struct oa_tc6 *tc6)
>> +{
>> +     bool do_tx_again;
>> +     u16 total_len;
>> +     u16 rca_len;
>> +     u16 tx_len;
>> +     int ret;
>> +
>> +     do {
>> +             do_tx_again = false;
>> +             rca_len = 0;
>> +             tx_len = 0;
>> +
>> +             /* In case of an interrupt, perform an empty chunk transfer to
>> +              * know the purpose of the interrupt. Interrupt may occur in
>> +              * case of RCA (Receive Chunk Available) and TXC (Transmit
>> +              * Credit Count). Both will occur if they are not indicated
>> +              * through the previous footer.
>> +              */
>> +             if (tc6->int_flag) {
>> +                     tc6->int_flag = false;
>> +                     total_len = oa_tc6_prepare_empty_chunk(tc6,
>> +                                                            tc6->spi_tx_buf,
>> +                                                            1);
>> +             } else {
>> +                     /* Calculate the transfer length */
>> +                     if (tc6->tx_flag && tc6->txc) {
>> +                             tx_len = oa_tc6_calculate_tx_len(tc6);
>> +                             memcpy(&tc6->spi_tx_buf[0],
>> +                                    &tc6->eth_tx_buf[tc6->tx_pos], tx_len);
>> +                     }
>> +
>> +                     if (tc6->rca)
>> +                             rca_len = oa_tc6_calculate_rca_len(tc6, tx_len);
>> +
>> +                     total_len = tx_len + rca_len;
>> +             }
>> +             ret = oa_tc6_spi_transfer(tc6->spi, tc6->spi_tx_buf,
>> +                                       tc6->spi_rx_buf, total_len);
>> +             if (ret)
>> +                     return ret;
>> +             /* Process the rxd chunks to get the ethernet frame or status */
>> +             ret = oa_tc6_process_rx_chunks(tc6, tc6->spi_rx_buf, total_len);
>> +             if (ret)
>> +                     return ret;
>> +             if (tc6->tx_flag) {
>> +                     tc6->tx_pos += tx_len;
>> +                     tc6->txc_needed = tc6->txc_needed -
>> +                                       (tx_len / (tc6->cps + TC6_HDR_SIZE));
>> +                     /* If the complete ethernet frame is transmitted then
>> +                      * return the skb and update the details to n/w layer.
>> +                      */
>> +                     if (!tc6->txc_needed)
>> +                             oa_tc6_tx_eth_complete(tc6);
>> +                     else if (tc6->txc)
>> +                             /* If txc is available again and updated from
>> +                              * the previous footer then perform tx again.
>> +                              */
>> +                             do_tx_again = true;
>> +             }
>> +
>> +             /* If rca is updated from the previous footer then perform empty
>> +              * tx to receive ethernet frame.
>> +              */
>> +             if (tc6->rca)
>> +                     do_tx_again = true;
>> +     } while (do_tx_again);
> 
> The coding standard say:
> 
> Functions should be short and sweet, and do just one thing. They
> should fit on one or two screenfuls of text (the ISO/ANSI screen size
> is 80x24, as we all know), and do one thing and do that well.
> 
> This is too long, and does too many things.
Sure, will split into multiple small functions.

Best Regards,
Parthiban V
> 
>       Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
index a4532c83e909..1306ca0b0884 100644
--- a/drivers/net/ethernet/oa_tc6.c
+++ b/drivers/net/ethernet/oa_tc6.c
@@ -14,17 +14,36 @@ 
 
 /* Opaque structure for MACPHY drivers */
 struct oa_tc6 {
+	int (*config_cps_buf)(void *tc6, u32 cps);
+	struct work_struct tx_work;
 	struct net_device *netdev;
 	struct phy_device *phydev;
 	struct mii_bus *mdiobus;
 	struct spi_device *spi;
+	struct sk_buff *tx_skb;
+	bool rx_eth_started;
 	struct device *dev;
+	/* Protects oa_tc6_perform_spi_xfer function elements between MAC-PHY
+	 * interrupt handler and the tx work handler.
+	 */
+	struct mutex lock;
 	u8 *ctrl_tx_buf;
 	u8 *ctrl_rx_buf;
+	u8 *spi_tx_buf;
+	u8 *spi_rx_buf;
+	u8 *eth_tx_buf;
+	u8 *eth_rx_buf;
+	u16 rxd_bytes;
+	u8 txc_needed;
+	bool int_flag;
+	bool tx_flag;
 	bool dprac;
 	bool iprac;
 	bool prote;
+	u16 tx_pos;
 	u32 cps;
+	u8 txc;
+	u8 rca;
 };
 
 static int oa_tc6_spi_transfer(struct spi_device *spi, u8 *ptx, u8 *prx, u16 len)
@@ -55,6 +74,24 @@  static int oa_tc6_get_parity(u32 p)
 	return !((p >> 28) & 1);
 }
 
+static u16 oa_tc6_prepare_empty_chunk(struct oa_tc6 *tc6, u8 *buf, u8 cp_count)
+{
+	u32 hdr;
+
+	/* Prepare empty chunks used for getting interrupt information or if
+	 * receive data available.
+	 */
+	for (u8 i = 0; i < cp_count; i++) {
+		hdr = FIELD_PREP(DATA_HDR_DNC, 1);
+		hdr |= FIELD_PREP(DATA_HDR_P, oa_tc6_get_parity(hdr));
+		*(__be32 *)&buf[i * (tc6->cps + TC6_HDR_SIZE)] = cpu_to_be32(hdr);
+		memset(&buf[TC6_HDR_SIZE + (i * (tc6->cps + TC6_HDR_SIZE))], 0,
+		       tc6->cps);
+	}
+
+	return cp_count * (tc6->cps + TC6_HDR_SIZE);
+}
+
 static void oa_tc6_prepare_ctrl_buf(struct oa_tc6 *tc6, u32 addr, u32 val[],
 				    u8 len, bool wnr, u8 *buf, bool prote)
 {
@@ -218,6 +255,14 @@  static int oa_tc6_configure(struct oa_tc6 *tc6)
 	bool ctc;
 	int ret;
 
+	/* Read BUFSTS register to get the current txc and rca. */
+	ret = oa_tc6_read_register(tc6, OA_TC6_BUFSTS, &regval);
+	if (ret)
+		return ret;
+
+	tc6->txc = FIELD_GET(TXC, regval);
+	tc6->rca = FIELD_GET(RCA, regval);
+
 	/* Read and configure the IMASK0 register for unmasking the interrupts */
 	ret = oa_tc6_perform_ctrl(tc6, IMASK0, &regval, 1, false, false);
 	if (ret)
@@ -259,6 +304,12 @@  static int oa_tc6_configure(struct oa_tc6 *tc6)
 		} else {
 			tc6->cps = OA_TC6_MAX_CPS;
 		}
+		/* Call queue buffer size config function if defined by MAC */
+		if (tc6->config_cps_buf) {
+			ret = tc6->config_cps_buf(tc6, tc6->cps);
+			if (ret)
+				return ret;
+		}
 		if (of_property_present(oa_node, "oa-txcte")) {
 			/* Return error if the tx cut through mode is configured
 			 * but it is not supported by MAC-PHY.
@@ -498,6 +549,240 @@  static int oa_tc6_phy_init(struct oa_tc6 *tc6)
 	return 0;
 }
 
+static int oa_tc6_process_exst(struct oa_tc6 *tc6)
+{
+	u32 regval;
+	int ret;
+
+	ret = oa_tc6_read_register(tc6, STATUS0, &regval);
+	if (ret)
+		return ret;
+
+	if (regval & TXPE)
+		net_err_ratelimited("%s: Transmit protocol error\n",
+				    tc6->netdev->name);
+
+	if (regval & TXBOE)
+		net_err_ratelimited("%s: Transmit buffer overflow\n",
+				    tc6->netdev->name);
+
+	if (regval & TXBUE)
+		net_err_ratelimited("%s: Transmit buffer underflow\n",
+				    tc6->netdev->name);
+
+	if (regval & RXBOE)
+		net_err_ratelimited("%s: Receive buffer overflow\n",
+				    tc6->netdev->name);
+
+	if (regval & LOFE)
+		net_err_ratelimited("%s: Loss of frame\n", tc6->netdev->name);
+
+	if (regval & HDRE)
+		net_err_ratelimited("%s: Header error\n", tc6->netdev->name);
+
+	if (regval & TXFCSE)
+		net_err_ratelimited("%s: Tx Frame Check Seq Error\n",
+				    tc6->netdev->name);
+
+	return oa_tc6_write_register(tc6, STATUS0, regval);
+}
+
+static void oa_tc6_rx_eth_ready(struct oa_tc6 *tc6)
+{
+	struct sk_buff *skb;
+
+	/* Send the received ethernet packet to network layer */
+	skb = netdev_alloc_skb(tc6->netdev, tc6->rxd_bytes + NET_IP_ALIGN);
+	if (!skb) {
+		tc6->netdev->stats.rx_dropped++;
+		netdev_dbg(tc6->netdev, "Out of memory for rx'd frame");
+	} else {
+		skb_reserve(skb, NET_IP_ALIGN);
+		memcpy(skb_put(skb, tc6->rxd_bytes), &tc6->eth_rx_buf[0],
+		       tc6->rxd_bytes);
+		skb->protocol = eth_type_trans(skb, tc6->netdev);
+		tc6->netdev->stats.rx_packets++;
+		tc6->netdev->stats.rx_bytes += tc6->rxd_bytes;
+		/* 0 for NET_RX_SUCCESS and 1 for NET_RX_DROP */
+		if (netif_rx(skb))
+			tc6->netdev->stats.rx_dropped++;
+	}
+}
+
+static void oa_tc6_rx_eth_complete2(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
+{
+	u16 ebo;
+
+	if (FIELD_GET(DATA_FTR_EV, ftr))
+		ebo = FIELD_GET(DATA_FTR_EBO, ftr) + 1;
+	else
+		ebo = tc6->cps;
+
+	memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[0], ebo);
+	tc6->rxd_bytes += ebo;
+	if (FIELD_GET(DATA_FTR_EV, ftr)) {
+		/* If EV set then send the received ethernet frame to n/w */
+		oa_tc6_rx_eth_ready(tc6);
+		tc6->rxd_bytes = 0;
+		tc6->rx_eth_started = false;
+	}
+}
+
+static void oa_tc6_rx_eth_complete1(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
+{
+	u16 ebo;
+	u16 sbo;
+
+	sbo = FIELD_GET(DATA_FTR_SWO, ftr) * 4;
+	ebo = FIELD_GET(DATA_FTR_EBO, ftr) + 1;
+
+	if (ebo <= sbo) {
+		memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[0], ebo);
+		tc6->rxd_bytes += ebo;
+		oa_tc6_rx_eth_ready(tc6);
+		tc6->rxd_bytes = 0;
+		memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo],
+		       tc6->cps - sbo);
+		tc6->rxd_bytes += (tc6->cps - sbo);
+	} else {
+		memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo],
+		       ebo - sbo);
+		tc6->rxd_bytes += (ebo - sbo);
+		oa_tc6_rx_eth_ready(tc6);
+		tc6->rxd_bytes = 0;
+	}
+}
+
+static void oa_tc6_start_rx_eth(struct oa_tc6 *tc6, u8 *payload, u32 ftr)
+{
+	u16 sbo;
+
+	tc6->rxd_bytes = 0;
+	tc6->rx_eth_started = true;
+	sbo = FIELD_GET(DATA_FTR_SWO, ftr) * 4;
+	memcpy(&tc6->eth_rx_buf[tc6->rxd_bytes], &payload[sbo], tc6->cps - sbo);
+	tc6->rxd_bytes += (tc6->cps - sbo);
+}
+
+static u32 oa_tc6_get_footer(struct oa_tc6 *tc6, u8 *buf, u8 cp_num)
+{
+	__be32 ftr;
+
+	ftr = *(__be32 *)&buf[tc6->cps + (cp_num * (tc6->cps + TC6_FTR_SIZE))];
+
+	return be32_to_cpu(ftr);
+}
+
+static void oa_tc6_update_txc_rca(struct oa_tc6 *tc6, u32 ftr)
+{
+	tc6->txc = FIELD_GET(DATA_FTR_TXC, ftr);
+	tc6->rca = FIELD_GET(DATA_FTR_RCA, ftr);
+}
+
+static int oa_tc6_check_ftr_errors(struct oa_tc6 *tc6, u32 ftr)
+{
+	/* Check for footer parity error */
+	if (oa_tc6_get_parity(ftr)) {
+		net_err_ratelimited("%s: Footer parity error\n",
+				    tc6->netdev->name);
+		return FTR_ERR;
+	}
+	/* If EXST set in the footer then read STS0 register to get the
+	 * status information.
+	 */
+	if (FIELD_GET(DATA_FTR_EXST, ftr)) {
+		if (oa_tc6_process_exst(tc6))
+			net_err_ratelimited("%s: Failed to process EXST\n",
+					    tc6->netdev->name);
+		return FTR_ERR;
+	}
+	if (FIELD_GET(DATA_FTR_HDRB, ftr)) {
+		net_err_ratelimited("%s: Footer eeceived header bad\n",
+				    tc6->netdev->name);
+		return FTR_ERR;
+	}
+	if (!FIELD_GET(DATA_FTR_SYNC, ftr)) {
+		net_err_ratelimited("%s: Footer configuration unsync\n",
+				    tc6->netdev->name);
+		return FTR_ERR;
+	}
+	return FTR_OK;
+}
+
+static void oa_tc6_drop_rx_eth(struct oa_tc6 *tc6)
+{
+	tc6->rxd_bytes = 0;
+	tc6->rx_eth_started = false;
+	tc6->netdev->stats.rx_dropped++;
+	net_err_ratelimited("%s: Footer frame drop\n",
+			    tc6->netdev->name);
+}
+
+static int oa_tc6_process_rx_chunks(struct oa_tc6 *tc6, u8 *buf, u16 len)
+{
+	u8 cp_count;
+	u8 *payload;
+	u32 ftr;
+	int ret;
+
+	/* Calculate the number of chunks received */
+	cp_count = len / (tc6->cps + TC6_FTR_SIZE);
+
+	for (u8 i = 0; i < cp_count; i++) {
+		/* Get the footer and payload */
+		ftr = oa_tc6_get_footer(tc6, buf, i);
+		payload = &buf[(i * (tc6->cps + TC6_FTR_SIZE))];
+		/* Check for footer errors */
+		ret = oa_tc6_check_ftr_errors(tc6, ftr);
+		if (ret) {
+			if (tc6->rx_eth_started)
+				oa_tc6_drop_rx_eth(tc6);
+			return ret;
+		}
+		/* If Frame Drop is set, indicates that the MAC has detected a
+		 * condition for which the SPI host should drop the received
+		 * ethernet frame.
+		 */
+		if (FIELD_GET(DATA_FTR_FD, ftr) && FIELD_GET(DATA_FTR_EV, ftr)) {
+			if (tc6->rx_eth_started)
+				oa_tc6_drop_rx_eth(tc6);
+
+			if (FIELD_GET(DATA_FTR_SV, ftr)) {
+				oa_tc6_start_rx_eth(tc6, payload, ftr);
+				oa_tc6_update_txc_rca(tc6, ftr);
+			}
+			continue;
+		}
+		/* Check for data valid */
+		if (FIELD_GET(DATA_FTR_DV, ftr)) {
+			/* Check whether both start valid and end valid are in a
+			 * single chunk payload means a single chunk payload may
+			 * contain an entire ethernet frame.
+			 */
+			if (FIELD_GET(DATA_FTR_SV, ftr) &&
+			    FIELD_GET(DATA_FTR_EV, ftr)) {
+				oa_tc6_rx_eth_complete1(tc6, payload, ftr);
+				oa_tc6_update_txc_rca(tc6, ftr);
+				continue;
+			}
+			/* Check for start valid to start capturing the incoming
+			 * ethernet frame.
+			 */
+			if (FIELD_GET(DATA_FTR_SV, ftr) && !tc6->rx_eth_started) {
+				oa_tc6_start_rx_eth(tc6, payload, ftr);
+				oa_tc6_update_txc_rca(tc6, ftr);
+				continue;
+			}
+
+			/* Check for end valid and calculate the copy length */
+			if (tc6->rx_eth_started)
+				oa_tc6_rx_eth_complete2(tc6, payload, ftr);
+		}
+		oa_tc6_update_txc_rca(tc6, ftr);
+	}
+	return FTR_OK;
+}
+
 static void oa_tc6_phy_exit(struct oa_tc6 *tc6)
 {
 	phy_disconnect(tc6->phydev);
@@ -505,17 +790,237 @@  static void oa_tc6_phy_exit(struct oa_tc6 *tc6)
 	mdiobus_free(tc6->mdiobus);
 }
 
+static void oa_tc6_prepare_tx_chunks(struct oa_tc6 *tc6, u8 *buf,
+				     struct sk_buff *skb)
+{
+	bool frame_started = false;
+	u16 copied_bytes = 0;
+	u16 copy_len;
+	u32 hdr;
+
+	/* Calculate the number tx credit counts needed to transport the tx
+	 * ethernet frame.
+	 */
+	tc6->txc_needed = (skb->len / tc6->cps) + ((skb->len % tc6->cps) ? 1 : 0);
+
+	for (u8 i = 0; i < tc6->txc_needed; i++) {
+		/* Prepare the header for each chunks to be transmitted */
+		hdr = FIELD_PREP(DATA_HDR_DNC, 1) |
+		      FIELD_PREP(DATA_HDR_DV, 1);
+		if (!frame_started) {
+			hdr |= FIELD_PREP(DATA_HDR_SV, 1) |
+			       FIELD_PREP(DATA_HDR_SWO, 0);
+			frame_started = true;
+		}
+		if ((tc6->cps + copied_bytes) >= skb->len) {
+			copy_len = skb->len - copied_bytes;
+			hdr |= FIELD_PREP(DATA_HDR_EBO, copy_len - 1) |
+			       FIELD_PREP(DATA_HDR_EV, 1);
+		} else {
+			copy_len = tc6->cps;
+		}
+		copied_bytes += copy_len;
+		hdr |= FIELD_PREP(DATA_HDR_P, oa_tc6_get_parity(hdr));
+		*(__be32 *)&buf[i * (tc6->cps + TC6_HDR_SIZE)] = cpu_to_be32(hdr);
+		/* Copy the ethernet frame in the chunk payload section */
+		memcpy(&buf[TC6_HDR_SIZE + (i * (tc6->cps + TC6_HDR_SIZE))],
+		       &skb->data[copied_bytes - copy_len], copy_len);
+	}
+}
+
+static u16 oa_tc6_calculate_tx_len(struct oa_tc6 *tc6)
+{
+	/* If the available txc is greater than the txc needed (calculated from
+	 * the tx ethernet frame then the tx length can be txc needed. Else the
+	 * tx length can be available txc and the remaining needed txc will be
+	 * updated either in the footer of the current transfer or through the
+	 * interrupt.
+	 */
+	if (tc6->txc >= tc6->txc_needed)
+		return tc6->txc_needed * (tc6->cps + TC6_HDR_SIZE);
+	else
+		return tc6->txc * (tc6->cps + TC6_HDR_SIZE);
+}
+
+static u16 oa_tc6_calculate_rca_len(struct oa_tc6 *tc6, u16 tx_len)
+{
+	u16 rca_needed = 0;
+	u16 tx_txc;
+
+	/* If tx eth frame and rca are available at the same time then check
+	 * whether the rca is less than the needed txc for the tx eth frame. If
+	 * not then add additional empty chunks along with the tx chunks to get
+	 * all the rca.
+	 */
+	if (tc6->tx_flag && tc6->txc) {
+		tx_txc = tc6->txc_needed - (tx_len / (tc6->cps + TC6_HDR_SIZE));
+		if (tx_txc < tc6->rca)
+			rca_needed = tc6->rca - tx_txc;
+	} else {
+		/* Add only empty chunks for rca if there is no tx chunks
+		 * available to transmit.
+		 */
+		rca_needed = tc6->rca;
+	}
+	return oa_tc6_prepare_empty_chunk(tc6, &tc6->spi_tx_buf[tx_len],
+					  rca_needed);
+}
+
+static void oa_tc6_tx_eth_complete(struct oa_tc6 *tc6)
+{
+	tc6->netdev->stats.tx_packets++;
+	tc6->netdev->stats.tx_bytes += tc6->tx_skb->len;
+	dev_kfree_skb(tc6->tx_skb);
+	tc6->tx_pos = 0;
+	tc6->tx_skb = NULL;
+	tc6->tx_flag = false;
+	if (netif_queue_stopped(tc6->netdev))
+		netif_wake_queue(tc6->netdev);
+}
+
+static int oa_tc6_perform_spi_xfer(struct oa_tc6 *tc6)
+{
+	bool do_tx_again;
+	u16 total_len;
+	u16 rca_len;
+	u16 tx_len;
+	int ret;
+
+	do {
+		do_tx_again = false;
+		rca_len = 0;
+		tx_len = 0;
+
+		/* In case of an interrupt, perform an empty chunk transfer to
+		 * know the purpose of the interrupt. Interrupt may occur in
+		 * case of RCA (Receive Chunk Available) and TXC (Transmit
+		 * Credit Count). Both will occur if they are not indicated
+		 * through the previous footer.
+		 */
+		if (tc6->int_flag) {
+			tc6->int_flag = false;
+			total_len = oa_tc6_prepare_empty_chunk(tc6,
+							       tc6->spi_tx_buf,
+							       1);
+		} else {
+			/* Calculate the transfer length */
+			if (tc6->tx_flag && tc6->txc) {
+				tx_len = oa_tc6_calculate_tx_len(tc6);
+				memcpy(&tc6->spi_tx_buf[0],
+				       &tc6->eth_tx_buf[tc6->tx_pos], tx_len);
+			}
+
+			if (tc6->rca)
+				rca_len = oa_tc6_calculate_rca_len(tc6, tx_len);
+
+			total_len = tx_len + rca_len;
+		}
+		ret = oa_tc6_spi_transfer(tc6->spi, tc6->spi_tx_buf,
+					  tc6->spi_rx_buf, total_len);
+		if (ret)
+			return ret;
+		/* Process the rxd chunks to get the ethernet frame or status */
+		ret = oa_tc6_process_rx_chunks(tc6, tc6->spi_rx_buf, total_len);
+		if (ret)
+			return ret;
+		if (tc6->tx_flag) {
+			tc6->tx_pos += tx_len;
+			tc6->txc_needed = tc6->txc_needed -
+					  (tx_len / (tc6->cps + TC6_HDR_SIZE));
+			/* If the complete ethernet frame is transmitted then
+			 * return the skb and update the details to n/w layer.
+			 */
+			if (!tc6->txc_needed)
+				oa_tc6_tx_eth_complete(tc6);
+			else if (tc6->txc)
+				/* If txc is available again and updated from
+				 * the previous footer then perform tx again.
+				 */
+				do_tx_again = true;
+		}
+
+		/* If rca is updated from the previous footer then perform empty
+		 * tx to receive ethernet frame.
+		 */
+		if (tc6->rca)
+			do_tx_again = true;
+	} while (do_tx_again);
+
+	return 0;
+}
+
+/* MAC-PHY interrupt handler */
+static irqreturn_t macphy_irq_handler(int irq, void *dev_id)
+{
+	struct oa_tc6 *tc6 = dev_id;
+
+	tc6->int_flag = true;
+	mutex_lock(&tc6->lock);
+	if (oa_tc6_perform_spi_xfer(tc6))
+		net_err_ratelimited("%s: SPI transfer failed\n",
+				    tc6->netdev->name);
+	mutex_unlock(&tc6->lock);
+
+	return IRQ_HANDLED;
+}
+
+/* Workqueue to perform SPI transfer */
+static void tc6_tx_work_handler(struct work_struct *work)
+{
+	struct oa_tc6 *tc6 = container_of(work, struct oa_tc6, tx_work);
+
+	mutex_lock(&tc6->lock);
+	if (oa_tc6_perform_spi_xfer(tc6))
+		net_err_ratelimited("%s: SPI transfer failed\n",
+				    tc6->netdev->name);
+	mutex_unlock(&tc6->lock);
+}
+
+/**
+ * oa_tc6_send_eth_pkt - function for sending the tx ethernet frame.
+ * @tc6: oa_tc6 struct.
+ * @skb: socket buffer in which the ethernet frame is stored.
+ *
+ * As this is called from atomic context, work queue is used here because the
+ * spi_sync will block for the transfer completion.
+ *
+ * Returns NETDEV_TX_OK if the tx work is scheduled or NETDEV_TX_BUSY if the
+ * previous enqueued tx skb is in progress.
+ */
+netdev_tx_t oa_tc6_send_eth_pkt(struct oa_tc6 *tc6, struct sk_buff *skb)
+{
+	if (tc6->tx_flag) {
+		netif_stop_queue(tc6->netdev);
+		return NETDEV_TX_BUSY;
+	}
+
+	tc6->tx_skb = skb;
+	/* Prepare tx chunks using the tx ethernet frame */
+	oa_tc6_prepare_tx_chunks(tc6, tc6->eth_tx_buf, skb);
+
+	tc6->tx_flag = true;
+	schedule_work(&tc6->tx_work);
+
+	return NETDEV_TX_OK;
+}
+EXPORT_SYMBOL_GPL(oa_tc6_send_eth_pkt);
+
 /**
  * oa_tc6_init - allocates and intializes oa_tc6 structure.
  * @spi: device with which data will be exchanged.
  * @netdev: network device to use.
+ * @config_cps_buf: function pointer passed by MAC driver to be called for
+ * configuring cps buffer size. Queue buffer size in the MAC has to be configured
+ * according to the cps.
  *
  * Returns pointer reference to the oa_tc6 structure if all the memory
  * allocation success otherwise NULL.
  */
-struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev)
+struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev,
+			   int (*config_cps_buf)(void *tc6, u32 cps))
 {
 	struct oa_tc6 *tc6;
+	int ret;
 
 	tc6 = devm_kzalloc(&spi->dev, sizeof(*tc6), GFP_KERNEL);
 	if (!tc6)
@@ -531,9 +1036,37 @@  struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev)
 	if (!tc6->ctrl_rx_buf)
 		return NULL;
 
+	/* Allocate memory for the tx buffer used for SPI transfer. */
+	tc6->spi_tx_buf = devm_kzalloc(&spi->dev, ETH_LEN + (OA_TC6_MAX_CPS * TC6_HDR_SIZE),
+				       GFP_KERNEL);
+	if (!tc6->spi_tx_buf)
+		return NULL;
+
+	/* Allocate memory for the rx buffer used for SPI transfer. */
+	tc6->spi_rx_buf = devm_kzalloc(&spi->dev, ETH_LEN + (OA_TC6_MAX_CPS * TC6_FTR_SIZE),
+				       GFP_KERNEL);
+	if (!tc6->spi_rx_buf)
+		return NULL;
+
+	/* Allocate memory for the tx ethernet chunks to transfer on SPI. */
+	tc6->eth_tx_buf = devm_kzalloc(&spi->dev, ETH_LEN + (OA_TC6_MAX_CPS * TC6_HDR_SIZE),
+				       GFP_KERNEL);
+	if (!tc6->eth_tx_buf)
+		return NULL;
+
+	/* Allocate memory for the rx ethernet packet. */
+	tc6->eth_rx_buf = devm_kzalloc(&spi->dev, ETH_LEN + (OA_TC6_MAX_CPS * TC6_FTR_SIZE),
+				       GFP_KERNEL);
+	if (!tc6->eth_rx_buf)
+		return NULL;
+
 	tc6->spi = spi;
 	tc6->netdev = netdev;
 	SET_NETDEV_DEV(netdev, &spi->dev);
+	tc6->config_cps_buf = config_cps_buf;
+	/* Set the SPI controller to pump at realtime priority */
+	spi->rt = true;
+	spi_setup(spi);
 
 	/* Perform MAC-PHY software reset */
 	if (oa_tc6_sw_reset(tc6)) {
@@ -552,6 +1085,16 @@  struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev)
 		dev_err(&spi->dev, "PHY initialization failed\n");
 		return NULL;
 	}
+	mutex_init(&tc6->lock);
+	INIT_WORK(&tc6->tx_work, tc6_tx_work_handler);
+	/* Register MAC-PHY interrupt service routine */
+	ret = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
+					macphy_irq_handler, IRQF_ONESHOT,
+					"macphy int", tc6);
+	if (ret) {
+		dev_err(&spi->dev, "Error attaching macphy irq %d\n", ret);
+		return NULL;
+	}
 
 	return tc6;
 }
@@ -564,6 +1107,7 @@  EXPORT_SYMBOL_GPL(oa_tc6_init);
  */
 void oa_tc6_exit(struct oa_tc6 *tc6)
 {
+	devm_free_irq(&tc6->spi->dev, tc6->spi->irq, tc6);
 	oa_tc6_phy_exit(tc6);
 }
 EXPORT_SYMBOL_GPL(oa_tc6_exit);
diff --git a/include/linux/oa_tc6.h b/include/linux/oa_tc6.h
index 36b729c384ac..58d1143f15ba 100644
--- a/include/linux/oa_tc6.h
+++ b/include/linux/oa_tc6.h
@@ -18,9 +18,39 @@ 
 #define CTRL_HDR_LEN	GENMASK(7, 1)	/* Length */
 #define CTRL_HDR_P	BIT(0)		/* Parity Bit */
 
+/* Data header */
+#define DATA_HDR_DNC	BIT(31)		/* Data-Not-Control */
+#define DATA_HDR_SEQ	BIT(30)		/* Data Chunk Sequence */
+#define DATA_HDR_NORX	BIT(29)		/* No Receive */
+#define DATA_HDR_DV	BIT(21)		/* Data Valid */
+#define DATA_HDR_SV	BIT(20)		/* Start Valid */
+#define DATA_HDR_SWO	GENMASK(19, 16)	/* Start Word Offset */
+#define DATA_HDR_EV	BIT(14)		/* End Valid */
+#define DATA_HDR_EBO	GENMASK(13, 8)	/* End Byte Offset */
+#define DATA_HDR_P	BIT(0)		/* Header Parity Bit */
+
+/* Data footer */
+#define DATA_FTR_EXST	BIT(31)		/* Extended Status */
+#define DATA_FTR_HDRB	BIT(30)		/* Received Header Bad */
+#define DATA_FTR_SYNC	BIT(29)		/* Configuration Synchronized */
+#define DATA_FTR_RCA	GENMASK(28, 24)	/* Receive Chunks Available */
+#define DATA_FTR_DV	BIT(21)		/* Data Valid */
+#define DATA_FTR_SV	BIT(20)		/* Start Valid */
+#define DATA_FTR_SWO	GENMASK(19, 16)	/* Start Word Offset */
+#define DATA_FTR_FD	BIT(15)		/* Frame Drop */
+#define DATA_FTR_EV	BIT(14)		/* End Valid */
+#define DATA_FTR_EBO	GENMASK(13, 8)	/* End Byte Offset */
+#define DATA_FTR_TXC	GENMASK(5, 1)	/* Transmit Credits */
+#define DATA_FTR_P	BIT(0)		/* Footer Parity Bit */
+
 #define TC6_HDR_SIZE		4	/* Ctrl command header size as per OA */
 #define TC6_FTR_SIZE		4	/* Ctrl command footer size ss per OA */
 #define TC6_CTRL_BUF_SIZE	1032	/* Max ctrl buffer size for 128 regs */
+
+#define FTR_OK		0
+#define FTR_ERR		1
+
+#define ETH_LEN		(ETH_DATA_LEN + ETH_HLEN + ETH_FCS_LEN)
 #define OA_TC6_MAX_CPS	64
 
 /* Open Alliance TC6 Standard Control and Status Registers */
@@ -45,7 +75,20 @@ 
 
 /* Status Register #0 */
 #define STATUS0		0x0008
+#define CDPE		BIT(12)	/* Control Data Protection Error */
+#define TXFCSE		BIT(11)	/* Transmit Frame Check Sequence Error */
 #define RESETC		BIT(6)	/* Reset Complete */
+#define HDRE		BIT(5)	/* Header Error */
+#define LOFE		BIT(4)	/* Loss of Framing Error */
+#define RXBOE		BIT(3)	/* Receive Buffer Overflow Error */
+#define TXBUE		BIT(2)	/* Transmit Buffer Underflow Error */
+#define TXBOE		BIT(1)	/* Transmit Buffer Overflow Error */
+#define TXPE		BIT(0)	/* Transmit Protocol Error */
+
+/* Buffer Status Register */
+#define OA_TC6_BUFSTS	0x000B
+#define TXC		GENMASK(15, 8)	/* Transmit Credits Available */
+#define RCA		GENMASK(7, 0)	/* Receive Chunks Available */
 
 /* Interrupt Mask Register #0 */
 #define IMASK0		0x000C
@@ -56,9 +99,11 @@ 
 #define TXBOEM		BIT(1)	/* Tx Buffer Overflow Error Mask */
 #define TXPEM		BIT(0)	/* Tx Protocol Error Mask */
 
-struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev);
+struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev,
+			   int (*config_cps_buf)(void *tc6, u32 cps));
 void oa_tc6_exit(struct oa_tc6 *tc6);
 int oa_tc6_write_register(struct oa_tc6 *tc6, u32 addr, u32 val);
 int oa_tc6_read_register(struct oa_tc6 *tc6, u32 addr, u32 *val);
 int oa_tc6_write_registers(struct oa_tc6 *tc6, u32 addr, u32 val[], u8 len);
 int oa_tc6_read_registers(struct oa_tc6 *tc6, u32 addr, u32 val[], u8 len);
+netdev_tx_t oa_tc6_send_eth_pkt(struct oa_tc6 *tc6, struct sk_buff *skb);