[PATCHv2 net-next 00/16] net: mvpp2: add basic support for PPv2.2

[PATCHv2 net-next 00/16] net: mvpp2: add basic support for PPv2.2

[Thomas Petazzoni]

Hello,

The goal of this patch series is to add basic support for PPv2.2 in
the existing mvpp2 driver. mvpp2 currently supported the PPv2.1
version of the IP, used in the 32 bits Marvell Armada 375 SoC. PPv2.2
is an evolution of this IP block, used in the 64 bits Marvell Armada
7K/8K SoCs.

In order to ease the review, the introduction of PPv2.2 support has
been made into multiple small commits, with the final commit adding
the compatible string that makes the PPv2.2 support actually
usable. The series remain fully bisectable.

This series depends on the series named "net: mvpp2: misc improvements
and preparation patches".

I'd like to thank Stefan Chulski and Marcin Wojtas, who helped me a
lot in the development of this patch series, by reviewing the patches,
and giving lots of useful hints to debug the driver on PPv2.2. Thanks!

Changes since v1:

 - Made a separate series from the set of patches doing preparation
   changes/fixes to the mvpp2 driver.

 - Rebased on top of v4.10-rc1.

 - Update Kconfig text of the mvpp2 driver to mention the support for
   Armada 7K and 8K (PPv2.2).

Best regards,

Thomas

Thomas Petazzoni (16):
  dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support
  net: mvpp2: add and use accessors for TX/RX descriptors
  net: mvpp2: add hw_version field in "struct mvpp2"
  net: mvpp2: introduce an intermediate union for the TX/RX descriptors
  net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors
  net: mvpp2: adjust the allocation/free of BM pools for PPv2.2
  net: mvpp2: adapt the mvpp2_rxq_*_pool_set functions to PPv2.2
  net: mvpp2: adapt mvpp2_defaults_set() to PPv2.2
  net: mvpp2: adjust mvpp2_{rxq,txq}_init for PPv2.2
  net: mvpp2: handle register mapping and access for PPv2.2
  net: mvpp2: handle misc PPv2.1/PPv2.2 differences
  net: mvpp2: add AXI bridge initialization for PPv2.2
  net: mvpp2: rework RXQ interrupt group initialization for PPv2.2
  net: mvpp2: adapt rxq distribution to PPv2.2
  net: mvpp2: add support for an additional clock needed for PPv2.2
  net: mvpp2: finally add the PPv2.2 compatible string

 .../devicetree/bindings/net/marvell-pp2.txt        |  66 +-
 drivers/net/ethernet/marvell/Kconfig               |   4 +-
 drivers/net/ethernet/marvell/mvpp2.c               | 783 +++++++++++++++++----
 3 files changed, 700 insertions(+), 153 deletions(-)

-- 
2.7.4

[PATCHv2 net-next 04/16] net: mvpp2: introduce an intermediate union for the TX/RX descriptors

[Thomas Petazzoni]

Since the format of the HW descriptors is different between PPv2.1 and
PPv2.2, this commit introduces an intermediate union, with for now
only the PPv2.1 descriptors. The bulk of the driver code only
manipulates opaque mvpp2_tx_desc and mvpp2_rx_desc pointers, and the
descriptors can only be accessed and modified through the accessor
functions. A follow-up commit will add the descriptor definitions for
PPv2.2.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 43 +++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index bc359a9..a37ff50 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -745,7 +745,8 @@ struct mvpp2_port {
 #define MVPP2_RXD_L3_IP6		BIT(30)
 #define MVPP2_RXD_BUF_HDR		BIT(31)
 
-struct mvpp2_tx_desc {
+/* HW TX descriptor for PPv2.1 */
+struct mvpp21_tx_desc {
 	u32 command;		/* Options used by HW for packet transmitting.*/
 	u8  packet_offset;	/* the offset from the buffer beginning	*/
 	u8  phys_txq;		/* destination queue ID			*/
@@ -756,7 +757,8 @@ struct mvpp2_tx_desc {
 	u32 reserved2;		/* reserved (for future use)		*/
 };
 
-struct mvpp2_rx_desc {
+/* HW RX descriptor for PPv2.1 */
+struct mvpp21_rx_desc {
 	u32 status;		/* info about received packet		*/
 	u16 reserved1;		/* parser_info (for future use, PnC)	*/
 	u16 data_size;		/* size of received packet in bytes	*/
@@ -771,6 +773,21 @@ struct mvpp2_rx_desc {
 	u32 reserved8;
 };
 
+/* Opaque type used by the driver to manipulate the HW TX and RX
+ * descriptors
+ */
+struct mvpp2_tx_desc {
+	union {
+		struct mvpp21_tx_desc pp21;
+	};
+};
+
+struct mvpp2_rx_desc {
+	union {
+		struct mvpp21_rx_desc pp21;
+	};
+};
+
 struct mvpp2_txq_pcpu_buf {
 	/* Transmitted SKB */
 	struct sk_buff *skb;
@@ -974,72 +991,72 @@ static u32 mvpp2_read(struct mvpp2 *priv, u32 offset)
 static dma_addr_t mvpp2_txdesc_phys_addr_get(struct mvpp2_port *port,
 					     struct mvpp2_tx_desc *tx_desc)
 {
-	return tx_desc->buf_phys_addr;
+	return tx_desc->pp21.buf_phys_addr;
 }
 
 static void mvpp2_txdesc_phys_addr_set(struct mvpp2_port *port,
 				       struct mvpp2_tx_desc *tx_desc,
 				       dma_addr_t phys_addr)
 {
-	tx_desc->buf_phys_addr = phys_addr;
+	tx_desc->pp21.buf_phys_addr = phys_addr;
 }
 
 static size_t mvpp2_txdesc_size_get(struct mvpp2_port *port,
 				    struct mvpp2_tx_desc *tx_desc)
 {
-	return tx_desc->data_size;
+	return tx_desc->pp21.data_size;
 }
 
 static void mvpp2_txdesc_size_set(struct mvpp2_port *port,
 				  struct mvpp2_tx_desc *tx_desc,
 				  size_t size)
 {
-	tx_desc->data_size = size;
+	tx_desc->pp21.data_size = size;
 }
 
 static void mvpp2_txdesc_txq_set(struct mvpp2_port *port,
 				 struct mvpp2_tx_desc *tx_desc,
 				 unsigned int txq)
 {
-	tx_desc->phys_txq = txq;
+	tx_desc->pp21.phys_txq = txq;
 }
 
 static void mvpp2_txdesc_cmd_set(struct mvpp2_port *port,
 				 struct mvpp2_tx_desc *tx_desc,
 				 unsigned int command)
 {
-	tx_desc->command = command;
+	tx_desc->pp21.command = command;
 }
 
 static void mvpp2_txdesc_offset_set(struct mvpp2_port *port,
 				    struct mvpp2_tx_desc *tx_desc,
 				    unsigned int offset)
 {
-	tx_desc->packet_offset = offset;
+	tx_desc->pp21.packet_offset = offset;
 }
 
 static dma_addr_t mvpp2_rxdesc_phys_addr_get(struct mvpp2_port *port,
 					     struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->buf_phys_addr;
+	return rx_desc->pp21.buf_phys_addr;
 }
 
 static unsigned long mvpp2_rxdesc_virt_addr_get(struct mvpp2_port *port,
 						struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->buf_cookie;
+	return rx_desc->pp21.buf_cookie;
 }
 
 static size_t mvpp2_rxdesc_size_get(struct mvpp2_port *port,
 				    struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->data_size;
+	return rx_desc->pp21.data_size;
 }
 
 static u32 mvpp2_rxdesc_status_get(struct mvpp2_port *port,
 				   struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->status;
+	return rx_desc->pp21.status;
 }
 
 static void mvpp2_txq_inc_get(struct mvpp2_txq_pcpu *txq_pcpu)
-- 
2.7.4

[PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Thomas Petazzoni]

This commit adds the definition of the PPv2.2 HW descriptors, adjusts
the mvpp2_tx_desc and mvpp2_rx_desc structures accordingly, and adapts
the accessors to work on both PPv2.1 and PPv2.2.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 109 +++++++++++++++++++++++++++++++----
 1 file changed, 98 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index a37ff50..0e00ec0 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -773,18 +773,42 @@ struct mvpp21_rx_desc {
 	u32 reserved8;
 };
 
+/* HW TX descriptor for PPv2.2 */
+struct mvpp22_tx_desc {
+	u32 command;
+	u8  packet_offset;
+	u8  phys_txq;
+	u16 data_size;
+	u64 reserved1;
+	u64 buf_phys_addr_ptp;
+	u64 buf_cookie_misc;
+};
+
+/* HW RX descriptor for PPv2.2 */
+struct mvpp22_rx_desc {
+	u32 status;
+	u16 reserved1;
+	u16 data_size;
+	u32 reserved2;
+	u32 reserved3;
+	u64 buf_phys_addr_key_hash;
+	u64 buf_cookie_misc;
+};
+
 /* Opaque type used by the driver to manipulate the HW TX and RX
  * descriptors
  */
 struct mvpp2_tx_desc {
 	union {
 		struct mvpp21_tx_desc pp21;
+		struct mvpp22_tx_desc pp22;
 	};
 };
 
 struct mvpp2_rx_desc {
 	union {
 		struct mvpp21_rx_desc pp21;
+		struct mvpp22_rx_desc pp22;
 	};
 };
 
@@ -991,72 +1015,135 @@ static u32 mvpp2_read(struct mvpp2 *priv, u32 offset)
 static dma_addr_t mvpp2_txdesc_phys_addr_get(struct mvpp2_port *port,
 					     struct mvpp2_tx_desc *tx_desc)
 {
-	return tx_desc->pp21.buf_phys_addr;
+	if (port->priv->hw_version == MVPP21)
+		return tx_desc->pp21.buf_phys_addr;
+	else
+		return tx_desc->pp22.buf_phys_addr_ptp & DMA_BIT_MASK(40);
 }
 
 static void mvpp2_txdesc_phys_addr_set(struct mvpp2_port *port,
 				       struct mvpp2_tx_desc *tx_desc,
 				       dma_addr_t phys_addr)
 {
-	tx_desc->pp21.buf_phys_addr = phys_addr;
+	if (port->priv->hw_version == MVPP21) {
+		tx_desc->pp21.buf_phys_addr = phys_addr;
+	} else {
+		u64 val = (u64)phys_addr;
+
+		tx_desc->pp22.buf_phys_addr_ptp &= ~DMA_BIT_MASK(40);
+		tx_desc->pp22.buf_phys_addr_ptp |= val;
+	}
 }
 
 static size_t mvpp2_txdesc_size_get(struct mvpp2_port *port,
 				    struct mvpp2_tx_desc *tx_desc)
 {
-	return tx_desc->pp21.data_size;
+	if (port->priv->hw_version == MVPP21)
+		return tx_desc->pp21.data_size;
+	else
+		return tx_desc->pp22.data_size;
 }
 
 static void mvpp2_txdesc_size_set(struct mvpp2_port *port,
 				  struct mvpp2_tx_desc *tx_desc,
 				  size_t size)
 {
-	tx_desc->pp21.data_size = size;
+	if (port->priv->hw_version == MVPP21)
+		tx_desc->pp21.data_size = size;
+	else
+		tx_desc->pp22.data_size = size;
 }
 
 static void mvpp2_txdesc_txq_set(struct mvpp2_port *port,
 				 struct mvpp2_tx_desc *tx_desc,
 				 unsigned int txq)
 {
-	tx_desc->pp21.phys_txq = txq;
+	if (port->priv->hw_version == MVPP21)
+		tx_desc->pp21.phys_txq = txq;
+	else
+		tx_desc->pp22.phys_txq = txq;
 }
 
 static void mvpp2_txdesc_cmd_set(struct mvpp2_port *port,
 				 struct mvpp2_tx_desc *tx_desc,
 				 unsigned int command)
 {
-	tx_desc->pp21.command = command;
+	if (port->priv->hw_version == MVPP21)
+		tx_desc->pp21.command = command;
+	else
+		tx_desc->pp22.command = command;
 }
 
 static void mvpp2_txdesc_offset_set(struct mvpp2_port *port,
 				    struct mvpp2_tx_desc *tx_desc,
 				    unsigned int offset)
 {
-	tx_desc->pp21.packet_offset = offset;
+	if (port->priv->hw_version == MVPP21)
+		tx_desc->pp21.packet_offset = offset;
+	else
+		tx_desc->pp22.packet_offset = offset;
 }
 
 static dma_addr_t mvpp2_rxdesc_phys_addr_get(struct mvpp2_port *port,
 					     struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->pp21.buf_phys_addr;
+	if (port->priv->hw_version == MVPP21)
+		return rx_desc->pp21.buf_phys_addr;
+	else
+		return rx_desc->pp22.buf_phys_addr_key_hash & DMA_BIT_MASK(40);
 }
 
 static unsigned long mvpp2_rxdesc_virt_addr_get(struct mvpp2_port *port,
 						struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->pp21.buf_cookie;
+	/* PPv2.1 can only be used on 32 bits architectures, and there
+	 * are 32 bits in buf_cookie which are enough to store the
+	 * full virtual address, so things are easy.
+	 */
+	if (port->priv->hw_version == MVPP21) {
+		return rx_desc->pp21.buf_cookie;
+	} else {
+		/* On PPv2.2, the situation is more complicated,
+		 * because there is only 40 bits to store the virtual
+		 * address, which is not sufficient. So on 64 bits
+		 * systems, we use phys_to_virt() to get the virtual
+		 * address from the physical address, which is fine
+		 * because the kernel linear mapping includes the
+		 * entire 40 bits physical address space. On 32 bits
+		 * systems however, we can't use phys_to_virt(), but
+		 * since virtual addresses are 32 bits only, there is
+		 * enough space in the RX descriptor for the full
+		 * virtual address.
+		 */
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+		dma_addr_t dma_addr =
+			rx_desc->pp22.buf_phys_addr_key_hash & DMA_BIT_MASK(40);
+		phys_addr_t phys_addr =
+			dma_to_phys(port->dev->dev.parent, dma_addr);
+
+		return (unsigned long)phys_to_virt(phys_addr);
+#else
+		return rx_desc->pp22.buf_cookie_misc & DMA_BIT_MASK(40);
+#endif
+	}
 }
 
 static size_t mvpp2_rxdesc_size_get(struct mvpp2_port *port,
 				    struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->pp21.data_size;
+	if (port->priv->hw_version == MVPP21)
+		return rx_desc->pp21.data_size;
+	else
+		return rx_desc->pp22.data_size;
 }
 
 static u32 mvpp2_rxdesc_status_get(struct mvpp2_port *port,
 				   struct mvpp2_rx_desc *rx_desc)
 {
-	return rx_desc->pp21.status;
+	if (port->priv->hw_version == MVPP21)
+		return rx_desc->pp21.status;
+	else
+		return rx_desc->pp22.status;
 }
 
 static void mvpp2_txq_inc_get(struct mvpp2_txq_pcpu *txq_pcpu)
-- 
2.7.4

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Russell King - ARM Linux]

On Wed, Dec 28, 2016 at 05:46:21PM +0100, Thomas Petazzoni wrote:
> This commit adds the definition of the PPv2.2 HW descriptors, adjusts
> the mvpp2_tx_desc and mvpp2_rx_desc structures accordingly, and adapts
> the accessors to work on both PPv2.1 and PPv2.2.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
...
> +		/* On PPv2.2, the situation is more complicated,
> +		 * because there is only 40 bits to store the virtual
> +		 * address, which is not sufficient. So on 64 bits
> +		 * systems, we use phys_to_virt() to get the virtual
> +		 * address from the physical address, which is fine
> +		 * because the kernel linear mapping includes the
> +		 * entire 40 bits physical address space. On 32 bits
> +		 * systems however, we can't use phys_to_virt(), but
> +		 * since virtual addresses are 32 bits only, there is
> +		 * enough space in the RX descriptor for the full
> +		 * virtual address.
> +		 */
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +		dma_addr_t dma_addr =
> +			rx_desc->pp22.buf_phys_addr_key_hash & DMA_BIT_MASK(40);
> +		phys_addr_t phys_addr =
> +			dma_to_phys(port->dev->dev.parent, dma_addr);
> +
> +		return (unsigned long)phys_to_virt(phys_addr);
> +#else
> +		return rx_desc->pp22.buf_cookie_misc & DMA_BIT_MASK(40);
> +#endif

I'm not sure that's the best way of selecting the difference.  It seems
that the issue here is the size of the virtual address, so why not test
the size of a virtual address pointer?

		if (8 * sizeof(rx_desc) > 40) {
			/* do phys addr dance */
		} else {
			return rx_desc->pp22.buf_cookie_misc & DMA_BIT_MASK(40);
		}

It also means that we get compile coverage over both sides of the
conditional.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Robin Murphy]

On 06/01/17 14:29, Russell King - ARM Linux wrote:
> On Wed, Dec 28, 2016 at 05:46:21PM +0100, Thomas Petazzoni wrote:
>> This commit adds the definition of the PPv2.2 HW descriptors, adjusts
>> the mvpp2_tx_desc and mvpp2_rx_desc structures accordingly, and adapts
>> the accessors to work on both PPv2.1 and PPv2.2.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ...
>> +		/* On PPv2.2, the situation is more complicated,
>> +		 * because there is only 40 bits to store the virtual
>> +		 * address, which is not sufficient. So on 64 bits
>> +		 * systems, we use phys_to_virt() to get the virtual
>> +		 * address from the physical address, which is fine
>> +		 * because the kernel linear mapping includes the
>> +		 * entire 40 bits physical address space. On 32 bits
>> +		 * systems however, we can't use phys_to_virt(), but
>> +		 * since virtual addresses are 32 bits only, there is
>> +		 * enough space in the RX descriptor for the full
>> +		 * virtual address.
>> +		 */
>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> +		dma_addr_t dma_addr =
>> +			rx_desc->pp22.buf_phys_addr_key_hash & DMA_BIT_MASK(40);
>> +		phys_addr_t phys_addr =
>> +			dma_to_phys(port->dev->dev.parent, dma_addr);

Ugh, this looks bogus. dma_to_phys(), in the arm64 case at least, is
essentially a SWIOTLB internal helper function which has to be
implemented in architecture code because reasons. Calling it from a
driver is almost certainly wrong (it doesn't even exist on most
architectures). Besides, if this is really a genuine dma_addr_t obtained
from a DMA API call, you cannot infer it to be related to a CPU physical
address, or convertible to one at all.

>> +
>> +		return (unsigned long)phys_to_virt(phys_addr);
>> +#else
>> +		return rx_desc->pp22.buf_cookie_misc & DMA_BIT_MASK(40);
>> +#endif
> 
> I'm not sure that's the best way of selecting the difference.

Given that CONFIG_ARCH_DMA_ADDR_T_64BIT could be enabled on 32-bit LPAE
systems, indeed it definitely isn't.

Robin.

>  It seems
> that the issue here is the size of the virtual address, so why not test
> the size of a virtual address pointer?
> 
> 		if (8 * sizeof(rx_desc) > 40) {
> 			/* do phys addr dance */
> 		} else {
> 			return rx_desc->pp22.buf_cookie_misc & DMA_BIT_MASK(40);
> 		}
> 
> It also means that we get compile coverage over both sides of the
> conditional.
> 

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Thomas Petazzoni]

Hello,

On Fri, 6 Jan 2017 14:44:56 +0000, Robin Murphy wrote:

> >> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> >> +		dma_addr_t dma_addr =
> >> +			rx_desc->pp22.buf_phys_addr_key_hash & DMA_BIT_MASK(40);
> >> +		phys_addr_t phys_addr =
> >> +			dma_to_phys(port->dev->dev.parent, dma_addr);  
> 
> Ugh, this looks bogus. dma_to_phys(), in the arm64 case at least, is
> essentially a SWIOTLB internal helper function which has to be
> implemented in architecture code because reasons. Calling it from a
> driver is almost certainly wrong (it doesn't even exist on most
> architectures). Besides, if this is really a genuine dma_addr_t obtained
> from a DMA API call, you cannot infer it to be related to a CPU physical
> address, or convertible to one at all.

So do you have a better suggestion? The descriptors only have enough
space to store a 40-bit virtual address, which is not enough to fit the
virtual addresses used by Linux for SKBs. This is why I'm instead
relying on the fact that the descriptors can store the 40-bit physical
address, and convert it back to a virtual address, which should be fine
on ARM64 because the entire physical memory is part of the kernel linear
mapping.

> >> +		return (unsigned long)phys_to_virt(phys_addr);
> >> +#else
> >> +		return rx_desc->pp22.buf_cookie_misc & DMA_BIT_MASK(40);
> >> +#endif  
> > 
> > I'm not sure that's the best way of selecting the difference.  
> 
> Given that CONFIG_ARCH_DMA_ADDR_T_64BIT could be enabled on 32-bit LPAE
> systems, indeed it definitely isn't.

Russell proposal of testing the size of a virtual address
pointer instead would solve this I believe, correct?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Robin Murphy]

On 03/02/17 13:24, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 6 Jan 2017 14:44:56 +0000, Robin Murphy wrote:
> 
>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>> +		dma_addr_t dma_addr =
>>>> +			rx_desc->pp22.buf_phys_addr_key_hash & DMA_BIT_MASK(40);
>>>> +		phys_addr_t phys_addr =
>>>> +			dma_to_phys(port->dev->dev.parent, dma_addr);  
>>
>> Ugh, this looks bogus. dma_to_phys(), in the arm64 case at least, is
>> essentially a SWIOTLB internal helper function which has to be
>> implemented in architecture code because reasons. Calling it from a
>> driver is almost certainly wrong (it doesn't even exist on most
>> architectures). Besides, if this is really a genuine dma_addr_t obtained
>> from a DMA API call, you cannot infer it to be related to a CPU physical
>> address, or convertible to one at all.
> 
> So do you have a better suggestion? The descriptors only have enough
> space to store a 40-bit virtual address, which is not enough to fit the
> virtual addresses used by Linux for SKBs. This is why I'm instead
> relying on the fact that the descriptors can store the 40-bit physical
> address, and convert it back to a virtual address, which should be fine
> on ARM64 because the entire physical memory is part of the kernel linear
> mapping.

OK, that has nothing to do with DMA addresses then.

>>>> +		return (unsigned long)phys_to_virt(phys_addr);
>>>> +#else
>>>> +		return rx_desc->pp22.buf_cookie_misc & DMA_BIT_MASK(40);
>>>> +#endif  
>>>
>>> I'm not sure that's the best way of selecting the difference.  
>>
>> Given that CONFIG_ARCH_DMA_ADDR_T_64BIT could be enabled on 32-bit LPAE
>> systems, indeed it definitely isn't.
> 
> Russell proposal of testing the size of a virtual address
> pointer instead would solve this I believe, correct?

AFAICS, even that shouldn't really be necessary - for all VA/PA
combinations of 32/32, 32/40 and 64/40, storing virt_to_phys() of the
SKB VA won't overflow 40 bits, so a corresponding phys_to_virt() at the
other end can't go wrong either. If you really do want to special-case
things based on VA size, though, either CONFIG_64BIT or sizeof(void *)
would indeed be infinitely more useful than the unrelated DMA address
width - I know this driver's never going to run on SPARC64, but that's
one example of where the above logic would lead to precisely the
truncated VA it's trying to avoid.

Robin.

> 
> Thanks,
> 
> Thomas
> 

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Thomas Petazzoni]

Hello,

On Fri, 3 Feb 2017 14:05:13 +0000, Robin Murphy wrote:

> > So do you have a better suggestion? The descriptors only have enough
> > space to store a 40-bit virtual address, which is not enough to fit the
> > virtual addresses used by Linux for SKBs. This is why I'm instead
> > relying on the fact that the descriptors can store the 40-bit physical
> > address, and convert it back to a virtual address, which should be fine
> > on ARM64 because the entire physical memory is part of the kernel linear
> > mapping.  
> 
> OK, that has nothing to do with DMA addresses then.

Well, it has to do with DMA in the sense that those buffers are
mapped with dma_map_single(). So the address that is given to us by the
hardware as the "physical address of the RX buffer" is the one that we
have initially given to the hardware and was the result of 
dma_map_single().

> > Russell proposal of testing the size of a virtual address
> > pointer instead would solve this I believe, correct?  
> 
> AFAICS, even that shouldn't really be necessary - for all VA/PA
> combinations of 32/32, 32/40 and 64/40, storing virt_to_phys() of the
> SKB VA won't overflow 40 bits,

I'm already lost here. Why are you talking about virt_to_phys() ? See
above: we have the dma_addr_t returned from dma_map_single(), and we
need to find back the corresponding virtual address, because there is
not enough room in the HW descriptors to store a full 64-bit VA.

> so a corresponding phys_to_virt() at the other end can't go wrong
> either. If you really do want to special-case things based on VA
> size, though, either CONFIG_64BIT or sizeof(void *) would indeed be
> infinitely more useful than the unrelated DMA address width - I know
> this driver's never going to run on SPARC64, but that's one example
> of where the above logic would lead to precisely the truncated VA
> it's trying to avoid.

What is different on SPARC64 here?

The situation we have is the following:

 - On systems where VAs are 32-bit wide, we have enough room to store
   the VA in the HW descriptor. So when we receive a packet, the HW
   descriptor provides us directly with the VA of the network packet,
   and the DMA address of the packet. We can dma_unmap_single() the
   packet, and do its processing.

 - On systems where VAs are 64-bit wide, we don't have enough room to
   store the VA in the HW descriptor. However, on 64-bit systems, the
   entire physical memory is mapped in the kernel linear mapping, so
   phys_to_virt() is valid on any physical address. And we use this
   property to retrieve the full 64-bit VA using the DMA address that
   we get from the HW descriptor.

   Since what we get from the HW descriptor is a DMA address, that's
   why we're using phys_to_virt(dma_to_phys(...)).

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Robin Murphy]

On 03/02/17 15:02, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 3 Feb 2017 14:05:13 +0000, Robin Murphy wrote:
> 
>>> So do you have a better suggestion? The descriptors only have enough
>>> space to store a 40-bit virtual address, which is not enough to fit the
>>> virtual addresses used by Linux for SKBs. This is why I'm instead
>>> relying on the fact that the descriptors can store the 40-bit physical
>>> address, and convert it back to a virtual address, which should be fine
>>> on ARM64 because the entire physical memory is part of the kernel linear
>>> mapping.  
>>
>> OK, that has nothing to do with DMA addresses then.
> 
> Well, it has to do with DMA in the sense that those buffers are
> mapped with dma_map_single(). So the address that is given to us by the
> hardware as the "physical address of the RX buffer" is the one that we
> have initially given to the hardware and was the result of 
> dma_map_single().

Ah, right. DMA addresses are not physical addresses. They may look very
much alike on many platforms, but that cannot be relied upon. On some
platforms, some of the "address bits" actually encode things like bus
attributes. If the device is behind an IOMMU, the DMA address can be
entirely arbitrary, and the only way to convert it back to a CPU address
(physical or virtual) is to walk the IOMMU page table.

>>> Russell proposal of testing the size of a virtual address
>>> pointer instead would solve this I believe, correct?  
>>
>> AFAICS, even that shouldn't really be necessary - for all VA/PA
>> combinations of 32/32, 32/40 and 64/40, storing virt_to_phys() of the
>> SKB VA won't overflow 40 bits,
> 
> I'm already lost here. Why are you talking about virt_to_phys() ? See
> above: we have the dma_addr_t returned from dma_map_single(), and we
> need to find back the corresponding virtual address, because there is
> not enough room in the HW descriptors to store a full 64-bit VA.

That's the wrong approach. You need to keep track of the VA which you
gave to dma_map_single() in the first place; *that* can be packed into
40 bits with a round-trip through a CPU physical address, assuming that
this "VA" field in the hardware descriptor is in addition to the actual
DMA address (and that the hardware descriptor is something different to
the rx_desc struct to which you could presumably just add an extra pointer).

>> so a corresponding phys_to_virt() at the other end can't go wrong
>> either. If you really do want to special-case things based on VA
>> size, though, either CONFIG_64BIT or sizeof(void *) would indeed be
>> infinitely more useful than the unrelated DMA address width - I know
>> this driver's never going to run on SPARC64, but that's one example
>> of where the above logic would lead to precisely the truncated VA
>> it's trying to avoid.
> 
> What is different on SPARC64 here?

void * and phys_addr_t are 64-bit, but dma_addr_t is 32-bit. The more
realistic concern, though, is that this using dma_to_phys() - which is a
DMA-API-internal helper for SWIOTLB - will break the driver's
COMPILE_TEST dependency.

> The situation we have is the following:
> 
>  - On systems where VAs are 32-bit wide, we have enough room to store
>    the VA in the HW descriptor. So when we receive a packet, the HW
>    descriptor provides us directly with the VA of the network packet,
>    and the DMA address of the packet. We can dma_unmap_single() the
>    packet, and do its processing.
> 
>  - On systems where VAs are 64-bit wide, we don't have enough room to
>    store the VA in the HW descriptor. However, on 64-bit systems, the
>    entire physical memory is mapped in the kernel linear mapping, so
>    phys_to_virt() is valid on any physical address. And we use this
>    property to retrieve the full 64-bit VA using the DMA address that
>    we get from the HW descriptor.
> 
>    Since what we get from the HW descriptor is a DMA address, that's
>    why we're using phys_to_virt(dma_to_phys(...)).

Marvell's product page tells me that Armada7k/8k include an IOMMU; at
some point, somebody's surely going to want to use it, and any invalid
assumptions about physical addresses are going to go bang.  You need to
treat CPU and DMA addresses as entirely separate, but since any CPU
address used for streaming DMA - i.e. dma_map_*() - must be a valid
linear map address, virtual and physical are relatively interchangeable
if the latter works out more convenient.

Robin.

> 
> Best regards,
> 
> Thomas
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Russell King - ARM Linux]

On Fri, Feb 03, 2017 at 02:05:13PM +0000, Robin Murphy wrote:
> AFAICS, even that shouldn't really be necessary - for all VA/PA
> combinations of 32/32, 32/40 and 64/40, storing virt_to_phys() of the
> SKB VA won't overflow 40 bits, so a corresponding phys_to_virt() at the
> other end can't go wrong either.

Except for the detail that virt_to_phys()/phys_to_virt() is only defined
for the direct-mapped memory, not for highmem.  That matters a lot for
32-bit platforms.

Now for a bit of a whinge.  Reading through this code is rather annoying
because of what's called a "physical" address which is actually a DMA
address as far as the kernel is concerned - this makes it much harder
when thinking about this issue because it causes all sorts of confusion.
Please can the next revision of the patches start calling things by their
right name - a dma_addr_t is a DMA address, not a physical address, even
though _numerically_ it may be the same thing.  From the point of view
of the kernel, you must not do phys_to_virt() on a dma_addr_t address.
Thanks.

If we're going to start dealing with _real_ physical addresses, then
this is even more important to separate the concept of what's a physical
address and what's a DMA address in this driver.

Taking a step backwards...

How do DMA addresses and this cookie get into the receive ring - from what
I can see, the driver doesn't write these into the receive ring, it's the
hardware that writes it, and the only route I can see that they get there
is via writes performed in mvpp2_bm_pool_put().

Now, from what I can see, the "buf_virt_addr" comes from:

+static void *mvpp2_frag_alloc(const struct mvpp2_bm_pool *pool)
+{
+       if (likely(pool->frag_size <= PAGE_SIZE))
+               return netdev_alloc_frag(pool->frag_size);
+       else
+               return kmalloc(pool->frag_size, GFP_ATOMIC);
+}

via mvpp2_buf_alloc().

Both kmalloc() and netdev_alloc_frag() guarantee that the virtual
address will be in lowmem.

Given that, I would suggest changing mvpp2_bm_pool_put() as follows -
and this is where my point above about separating the notion of "dma
address" and "physical address" becomes very important:

 static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
-                                    dma_addr_t buf_phys_addr,
-                                    unsigned long buf_virt_addr)
+                                    dma_addr_t dma, phys_addr_t phys)
 {

and updating it to write "phys" as the existing buf_virt_addr.

In mvpp2_bm_bufs_add():

                buf = mvpp2_buf_alloc(port, bm_pool, &phys_addr, GFP_KERNEL);
                if (!buf)
                        break;
 
                mvpp2_bm_pool_put(port, bm_pool->id, phys_addr,
-                                 (unsigned long)buf);
+                                 virt_to_phys(buf));

which I think means that mvpp2_rxdesc_virt_addr_get() can just become:

 	phys_addr_t cookie;
 
        /* PPv2.1 can only be used on 32 bits architectures, and there
         * are 32 bits in buf_cookie which are enough to store the
         * full virtual address, so things are easy.
         */
        if (port->priv->hw_version == MVPP21)
                cookie = rx_desc->pp21.buf_cookie;
        else
 		cookie = rx_desc->pp22.buf_cookie_misc & FORTY_BIT_MASK;

 	return phys_to_virt(cookie);

I'd suggest against using DMA_BIT_MASK(40) there - because it's not a
DMA address, even though it happens to resolve to the same number.

Again, I may have missed how the addresses end up getting into
buf_cookie_misc, so what I suggest above may not be possible.

I'd also suggest that there is some test in mvpp2_bm_bufs_add() to
verify that the physical addresses and DMA addresses do fit within
the available number of bits - if they don't we could end up scribbling
over memory that we shouldn't be, and it looks like we have a failure
path there to gracefully handle that situation - gracefully compared
to a nasty BUG_ON().

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[Thomas Petazzoni]

Hello,

On Fri, 3 Feb 2017 15:54:49 +0000, Russell King - ARM Linux wrote:

> Now for a bit of a whinge.  Reading through this code is rather annoying
> because of what's called a "physical" address which is actually a DMA
> address as far as the kernel is concerned - this makes it much harder
> when thinking about this issue because it causes all sorts of confusion.
> Please can the next revision of the patches start calling things by their
> right name - a dma_addr_t is a DMA address, not a physical address, even
> though _numerically_ it may be the same thing.  From the point of view
> of the kernel, you must not do phys_to_virt() on a dma_addr_t address.
> Thanks.

I do know that phys_addr_t is different from dma_addr_t. The case where
you have an IOMMU makes this very obvious. The fact that the driver
isn't completely correct in that respect indeed needs to be fixed.

> Taking a step backwards...
> 
> How do DMA addresses and this cookie get into the receive ring - from what
> I can see, the driver doesn't write these into the receive ring, it's the
> hardware that writes it, and the only route I can see that they get there
> is via writes performed in mvpp2_bm_pool_put().

Correct. Here is a quick summary of my understand of the HW operation.
The HW has a "buffer manager", i.e it can internally manage a list of
buffers. At initialization time, we provide to the HW a list of buffers
by pushing the DMA address and virtual address of each buffer into a
register. Thanks to that the HW then knows about all the buffers we
have given. Then, upon RX of a packet, the HW picks one of those
buffers, and then fills a RX descriptor with the DMA address of the
packet, and the VA in the "cookie" field. The "cookie" field can I
believe be used for whatever we want, it's just "free" space in the
descriptor.

> Now, from what I can see, the "buf_virt_addr" comes from:
> 
> +static void *mvpp2_frag_alloc(const struct mvpp2_bm_pool *pool)
> +{
> +       if (likely(pool->frag_size <= PAGE_SIZE))
> +               return netdev_alloc_frag(pool->frag_size);
> +       else
> +               return kmalloc(pool->frag_size, GFP_ATOMIC);
> +}
> 
> via mvpp2_buf_alloc().
> 
> Both kmalloc() and netdev_alloc_frag() guarantee that the virtual
> address will be in lowmem.

*That* is the thing I didn't realize until now. If the buffers are
always in lowmem, then it's much easier because we can indeed use
virt_to_phys()/phys_to_virt() on them. But it's indeed obvious they are
in lowmem, since we get a void* pointer for them.

> Given that, I would suggest changing mvpp2_bm_pool_put() as follows -
> and this is where my point above about separating the notion of "dma
> address" and "physical address" becomes very important:
> 
>  static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
> -                                    dma_addr_t buf_phys_addr,
> -                                    unsigned long buf_virt_addr)
> +                                    dma_addr_t dma, phys_addr_t phys)
>  {
> 
> and updating it to write "phys" as the existing buf_virt_addr.
> 
> In mvpp2_bm_bufs_add():
> 
>                 buf = mvpp2_buf_alloc(port, bm_pool, &phys_addr, GFP_KERNEL);
>                 if (!buf)
>                         break;
>  
>                 mvpp2_bm_pool_put(port, bm_pool->id, phys_addr,
> -                                 (unsigned long)buf);
> +                                 virt_to_phys(buf));
> 
> which I think means that mvpp2_rxdesc_virt_addr_get() can just become:
> 
>  	phys_addr_t cookie;
>  
>         /* PPv2.1 can only be used on 32 bits architectures, and there
>          * are 32 bits in buf_cookie which are enough to store the
>          * full virtual address, so things are easy.
>          */
>         if (port->priv->hw_version == MVPP21)
>                 cookie = rx_desc->pp21.buf_cookie;
>         else
>  		cookie = rx_desc->pp22.buf_cookie_misc & FORTY_BIT_MASK;
> 
>  	return phys_to_virt(cookie);

This makes complete sense. We use the cookie to store the phys_addr_t
rather than the virtual address. I might be missing something, but it
seems like a very good solution. Thanks for the suggestion, I'll try
this!

> I'd suggest against using DMA_BIT_MASK(40) there - because it's not a
> DMA address, even though it happens to resolve to the same number.

Sure, will update this as well.

> Again, I may have missed how the addresses end up getting into
> buf_cookie_misc, so what I suggest above may not be possible.

No, I think you got it right. At least it matches my own understanding
of the HW.

> I'd also suggest that there is some test in mvpp2_bm_bufs_add() to
> verify that the physical addresses and DMA addresses do fit within
> the available number of bits - if they don't we could end up scribbling
> over memory that we shouldn't be, and it looks like we have a failure
> path there to gracefully handle that situation - gracefully compared
> to a nasty BUG_ON().

For the DMA addresses, I have some additional changes on top of my v2
to use dma_set_mask() to ensure that DMA addresses fit in 40 bits. But
that's only for DMA addresses indeed.

Thanks again for your suggestion!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

RE: [PATCHv2 net-next 05/16] net: mvpp2: introduce PPv2.2 HW descriptors and adapt accessors

[David Laight]

From: Thomas Petazzoni
> Sent: 04 February 2017 14:00
...
> This makes complete sense. We use the cookie to store the phys_addr_t
> rather than the virtual address. I might be missing something, but it
> seems like a very good solution. Thanks for the suggestion, I'll try
> this!

Why not just store an array index for the buffer info?
That would save you having to regenerate kernel virtual
addresses from limited info.

Or realise that the low bits (page offset) of the dma address
and kernel virtual address (and kernel physical for that matter)
will always match. 50 bits might be enough for a kernel virtual address.

	David

[PATCHv2 net-next 06/16] net: mvpp2: adjust the allocation/free of BM pools for PPv2.2

[Thomas Petazzoni]

This commit adjusts the allocation and freeing of BM pools to support
PPv2.2. This involves:

 - Checking that the number of buffer pointers is a multiple of 16, as
   required by the hardware.

 - Adjusting the size of the DMA coherent area allocated for buffer
   pointers. Indeed, PPv2.2 needs space for 2 pointers of 64-bits per
   buffer, as opposed to 2 pointers of 32-bits per buffer in
   PPv2.1. The size in bytes is now stored in a new field of the
   mvpp2_bm_pool structure.

 - On PPv2.2, the 32 high order bits of the BM pointer area physical
   address must be programmed in the MVPP2_BM_HIGH_BASE_REG register.

 - On PPv2.2, getting the physical and virtual address of each buffer
   requires reading the MVPP2_BM_ADDR_HIGH_ALLOC to get the high order
   bits of those addresses. A new utility function
   mvpp2_bm_bufs_get_addrs() is introduced to handle this.

 - On PPv2.2, releasing a buffer requires writing the high order 32 bits
   of the physical address to MVPP2_BM_PHY_VIRT_HIGH_RLS_REG. We no
   longer need to write the virtual address to MVPP2_BM_VIRT_RLS_REG.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 97 ++++++++++++++++++++++++++++++------
 1 file changed, 82 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 0e00ec0..160b787 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -208,17 +208,28 @@
 #define     MVPP2_BM_BPPE_FULL_MASK		BIT(3)
 #define     MVPP2_BM_AVAILABLE_BP_LOW_MASK	BIT(4)
 #define MVPP2_BM_INTR_MASK_REG(pool)		(0x6280 + ((pool) * 4))
+#define MVPP2_BM_HIGH_BASE_REG			0x6310
+#define     MVPP2_BM_HIGH_BASE_MASK		0xff
 #define MVPP2_BM_PHY_ALLOC_REG(pool)		(0x6400 + ((pool) * 4))
 #define     MVPP2_BM_PHY_ALLOC_GRNTD_MASK	BIT(0)
 #define MVPP2_BM_VIRT_ALLOC_REG			0x6440
+#define MVPP2_BM_ADDR_HIGH_ALLOC		0x6444
+#define     MVPP2_BM_ADDR_HIGH_PHYS_MASK	0xff
+#define     MVPP2_BM_ADDR_HIGH_VIRT_MASK	0xff00
+#define     MVPP2_BM_ADDR_HIGH_VIRT_SHIFT	8
 #define MVPP2_BM_PHY_RLS_REG(pool)		(0x6480 + ((pool) * 4))
 #define     MVPP2_BM_PHY_RLS_MC_BUFF_MASK	BIT(0)
 #define     MVPP2_BM_PHY_RLS_PRIO_EN_MASK	BIT(1)
 #define     MVPP2_BM_PHY_RLS_GRNTD_MASK		BIT(2)
 #define MVPP2_BM_VIRT_RLS_REG			0x64c0
-#define MVPP2_BM_MC_RLS_REG			0x64c4
+#define MVPP21_BM_MC_RLS_REG			0x64c4
 #define     MVPP2_BM_MC_ID_MASK			0xfff
 #define     MVPP2_BM_FORCE_RELEASE_MASK		BIT(12)
+#define MVPP22_BM_ADDR_HIGH_RLS_REG		0x64c4
+#define     MVPP22_BM_ADDR_HIGH_PHYS_RLS_MASK	0xff
+#define	    MVPP22_BM_ADDR_HIGH_VIRT_RLS_MASK	0xff00
+#define     MVPP22_BM_ADDR_HIGH_VIRT_RLS_SHIFT	8
+#define MVPP22_BM_MC_RLS_REG			0x64d4
 
 /* TX Scheduler registers */
 #define MVPP2_TXP_SCHED_PORT_INDEX_REG		0x8000
@@ -957,6 +968,8 @@ struct mvpp2_bm_pool {
 
 	/* Buffer Pointers Pool External (BPPE) size */
 	int size;
+	/* BPPE size in bytes */
+	int size_bytes;
 	/* Number of buffers for this pool */
 	int buf_num;
 	/* Pool buffer size */
@@ -3558,11 +3571,23 @@ static int mvpp2_bm_pool_create(struct platform_device *pdev,
 				struct mvpp2 *priv,
 				struct mvpp2_bm_pool *bm_pool, int size)
 {
-	int size_bytes;
 	u32 val;
 
-	size_bytes = sizeof(u32) * size;
-	bm_pool->virt_addr = dma_alloc_coherent(&pdev->dev, size_bytes,
+	/* Number of buffer pointers must be a multiple of 16, as per
+	 * hardware constraints
+	 */
+	if (!IS_ALIGNED(size, 16))
+		return -EINVAL;
+
+	/* PPv2.1 needs 8 bytes per buffer pointer, PPv2.2 needs 16
+	 * bytes per buffer pointer
+	 */
+	if (priv->hw_version == MVPP21)
+		bm_pool->size_bytes = 2 * sizeof(u32) * size;
+	else
+		bm_pool->size_bytes = 2 * sizeof(u64) * size;
+
+	bm_pool->virt_addr = dma_alloc_coherent(&pdev->dev, bm_pool->size_bytes,
 						&bm_pool->phys_addr,
 						GFP_KERNEL);
 	if (!bm_pool->virt_addr)
@@ -3570,15 +3595,24 @@ static int mvpp2_bm_pool_create(struct platform_device *pdev,
 
 	if (!IS_ALIGNED((unsigned long)bm_pool->virt_addr,
 			MVPP2_BM_POOL_PTR_ALIGN)) {
-		dma_free_coherent(&pdev->dev, size_bytes, bm_pool->virt_addr,
-				  bm_pool->phys_addr);
+		dma_free_coherent(&pdev->dev, bm_pool->size_bytes,
+				  bm_pool->virt_addr, bm_pool->phys_addr);
 		dev_err(&pdev->dev, "BM pool %d is not %d bytes aligned\n",
 			bm_pool->id, MVPP2_BM_POOL_PTR_ALIGN);
 		return -ENOMEM;
 	}
 
 	mvpp2_write(priv, MVPP2_BM_POOL_BASE_REG(bm_pool->id),
-		    bm_pool->phys_addr);
+		    lower_32_bits(bm_pool->phys_addr));
+	/* On PPv2.2, program the high order bits of the base address */
+	if (priv->hw_version == MVPP22) {
+		if (sizeof(dma_addr_t) == 8)
+			val = upper_32_bits(bm_pool->phys_addr) &
+				MVPP2_BM_HIGH_BASE_MASK;
+		else
+			val = 0;
+		mvpp2_write(priv, MVPP2_BM_HIGH_BASE_REG, val);
+	}
 	mvpp2_write(priv, MVPP2_BM_POOL_SIZE_REG(bm_pool->id), size);
 
 	val = mvpp2_read(priv, MVPP2_BM_POOL_CTRL_REG(bm_pool->id));
@@ -3606,6 +3640,27 @@ static void mvpp2_bm_pool_bufsize_set(struct mvpp2 *priv,
 	mvpp2_write(priv, MVPP2_POOL_BUF_SIZE_REG(bm_pool->id), val);
 }
 
+static void mvpp2_bm_bufs_get_addrs(struct device *dev, struct mvpp2 *priv,
+				    struct mvpp2_bm_pool *bm_pool,
+				    dma_addr_t *paddr, unsigned long *vaddr)
+{
+	*paddr = mvpp2_read(priv, MVPP2_BM_PHY_ALLOC_REG(bm_pool->id));
+	*vaddr = mvpp2_read(priv, MVPP2_BM_VIRT_ALLOC_REG);
+
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	if (priv->hw_version == MVPP22) {
+		u32 val;
+		u32 paddr_highbits;
+
+		val = mvpp2_read(priv, MVPP2_BM_ADDR_HIGH_ALLOC);
+		paddr_highbits = (val & MVPP2_BM_ADDR_HIGH_PHYS_MASK);
+
+		*paddr |= (dma_addr_t)paddr_highbits << 32;
+		*vaddr = (unsigned long)phys_to_virt(dma_to_phys(dev, *paddr));
+	}
+#endif
+}
+
 /* Free all buffers from the pool */
 static void mvpp2_bm_bufs_free(struct device *dev, struct mvpp2 *priv,
 			       struct mvpp2_bm_pool *bm_pool)
@@ -3616,10 +3671,8 @@ static void mvpp2_bm_bufs_free(struct device *dev, struct mvpp2 *priv,
 		dma_addr_t buf_phys_addr;
 		unsigned long vaddr;
 
-		/* Get buffer virtual address (indirect access) */
-		buf_phys_addr = mvpp2_read(priv,
-					   MVPP2_BM_PHY_ALLOC_REG(bm_pool->id));
-		vaddr = mvpp2_read(priv, MVPP2_BM_VIRT_ALLOC_REG);
+		mvpp2_bm_bufs_get_addrs(dev, priv, bm_pool,
+					&buf_phys_addr, &vaddr);
 
 		dma_unmap_single(dev, buf_phys_addr,
 				 bm_pool->buf_size, DMA_FROM_DEVICE);
@@ -3651,7 +3704,7 @@ static int mvpp2_bm_pool_destroy(struct platform_device *pdev,
 	val |= MVPP2_BM_STOP_MASK;
 	mvpp2_write(priv, MVPP2_BM_POOL_CTRL_REG(bm_pool->id), val);
 
-	dma_free_coherent(&pdev->dev, sizeof(u32) * bm_pool->size,
+	dma_free_coherent(&pdev->dev, bm_pool->size_bytes,
 			  bm_pool->virt_addr,
 			  bm_pool->phys_addr);
 	return 0;
@@ -3787,8 +3840,19 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
 				     dma_addr_t buf_phys_addr,
 				     unsigned long buf_virt_addr)
 {
-	mvpp2_write(port->priv, MVPP2_BM_VIRT_RLS_REG, buf_virt_addr);
-	mvpp2_write(port->priv, MVPP2_BM_PHY_RLS_REG(pool), buf_phys_addr);
+#if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT)
+	u32 val;
+
+	val = upper_32_bits(buf_phys_addr) & MVPP22_BM_ADDR_HIGH_PHYS_RLS_MASK;
+	val |= (upper_32_bits(buf_virt_addr) &
+		MVPP22_BM_ADDR_HIGH_VIRT_RLS_MASK)
+		<< MVPP22_BM_ADDR_HIGH_VIRT_RLS_SHIFT;
+	mvpp2_write(port->priv, MVPP22_BM_ADDR_HIGH_RLS_REG, val);
+#endif
+	mvpp2_write(port->priv, MVPP2_BM_VIRT_RLS_REG,
+		    lower_32_bits(buf_virt_addr));
+	mvpp2_write(port->priv, MVPP2_BM_PHY_RLS_REG(pool),
+		    lower_32_bits(buf_phys_addr));
 }
 
 /* Release multicast buffer */
@@ -3800,7 +3864,10 @@ static void mvpp2_bm_pool_mc_put(struct mvpp2_port *port, int pool,
 	u32 val = 0;
 
 	val |= (mc_id & MVPP2_BM_MC_ID_MASK);
-	mvpp2_write(port->priv, MVPP2_BM_MC_RLS_REG, val);
+	if (port->priv->hw_version == MVPP21)
+		mvpp2_write(port->priv, MVPP21_BM_MC_RLS_REG, val);
+	else
+		mvpp2_write(port->priv, MVPP22_BM_MC_RLS_REG, val);
 
 	mvpp2_bm_pool_put(port, pool,
 			  buf_phys_addr | MVPP2_BM_PHY_RLS_MC_BUFF_MASK,
-- 
2.7.4

Re: [PATCHv2 net-next 06/16] net: mvpp2: adjust the allocation/free of BM pools for PPv2.2

[Russell King - ARM Linux]

On Wed, Dec 28, 2016 at 05:46:22PM +0100, Thomas Petazzoni wrote:
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +	if (priv->hw_version == MVPP22) {

Maybe
	if (sizeof(dma_addr_t) > sizeof(u32) && priv->hw_version == MVPP22) {

to get better compile coverage?

> +		u32 val;
> +		u32 paddr_highbits;
> +
> +		val = mvpp2_read(priv, MVPP2_BM_ADDR_HIGH_ALLOC);
> +		paddr_highbits = (val & MVPP2_BM_ADDR_HIGH_PHYS_MASK);
> +
> +		*paddr |= (dma_addr_t)paddr_highbits << 32;
> +		*vaddr = (unsigned long)phys_to_virt(dma_to_phys(dev, *paddr));
> +	}
> +#endif
> +}
> +
>  /* Free all buffers from the pool */
>  static void mvpp2_bm_bufs_free(struct device *dev, struct mvpp2 *priv,
>  			       struct mvpp2_bm_pool *bm_pool)
> @@ -3616,10 +3671,8 @@ static void mvpp2_bm_bufs_free(struct device *dev, struct mvpp2 *priv,
>  		dma_addr_t buf_phys_addr;
>  		unsigned long vaddr;
>  
> -		/* Get buffer virtual address (indirect access) */
> -		buf_phys_addr = mvpp2_read(priv,
> -					   MVPP2_BM_PHY_ALLOC_REG(bm_pool->id));
> -		vaddr = mvpp2_read(priv, MVPP2_BM_VIRT_ALLOC_REG);
> +		mvpp2_bm_bufs_get_addrs(dev, priv, bm_pool,
> +					&buf_phys_addr, &vaddr);
>  
>  		dma_unmap_single(dev, buf_phys_addr,
>  				 bm_pool->buf_size, DMA_FROM_DEVICE);
> @@ -3651,7 +3704,7 @@ static int mvpp2_bm_pool_destroy(struct platform_device *pdev,
>  	val |= MVPP2_BM_STOP_MASK;
>  	mvpp2_write(priv, MVPP2_BM_POOL_CTRL_REG(bm_pool->id), val);
>  
> -	dma_free_coherent(&pdev->dev, sizeof(u32) * bm_pool->size,
> +	dma_free_coherent(&pdev->dev, bm_pool->size_bytes,
>  			  bm_pool->virt_addr,
>  			  bm_pool->phys_addr);
>  	return 0;
> @@ -3787,8 +3840,19 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
>  				     dma_addr_t buf_phys_addr,
>  				     unsigned long buf_virt_addr)
>  {
> -	mvpp2_write(port->priv, MVPP2_BM_VIRT_RLS_REG, buf_virt_addr);
> -	mvpp2_write(port->priv, MVPP2_BM_PHY_RLS_REG(pool), buf_phys_addr);
> +#if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT)
> +	u32 val;
> +
> +	val = upper_32_bits(buf_phys_addr) & MVPP22_BM_ADDR_HIGH_PHYS_RLS_MASK;
> +	val |= (upper_32_bits(buf_virt_addr) &
> +		MVPP22_BM_ADDR_HIGH_VIRT_RLS_MASK)
> +		<< MVPP22_BM_ADDR_HIGH_VIRT_RLS_SHIFT;
> +	mvpp2_write(port->priv, MVPP22_BM_ADDR_HIGH_RLS_REG, val);
> +#endif

Similar compile-time conditional?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Thomas Petazzoni]

The Marvell PPv2 Device Tree binding was so far only used to describe
the PPv2.1 network controller, used in the Marvell Armada 375.

A new version of this IP block, PPv2.2 is used in the Marvell Armada
7K/8K processor. This commit extends the existing binding so that it can
also be used to describe PPv2.2 hardware.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 .../devicetree/bindings/net/marvell-pp2.txt        | 66 ++++++++++++++++++----
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt
index aa4f423..76071f3 100644
--- a/Documentation/devicetree/bindings/net/marvell-pp2.txt
+++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt
@@ -1,17 +1,28 @@
-* Marvell Armada 375 Ethernet Controller (PPv2)
+* Marvell Armada 375 Ethernet Controller (PPv2.1)
+  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
 
 Required properties:
 
-- compatible: should be "marvell,armada-375-pp2"
+- compatible: should be one of:
+    "marvell,armada-375-pp2"
+    "marvell,armada-7k-pp2"
 - reg: addresses and length of the register sets for the device.
-  Must contain the following register sets:
+  For "marvell,armada-375-pp2", must contain the following register
+  sets:
 	- common controller registers
 	- LMS registers
-  In addition, at least one port register set is required.
-- clocks: a pointer to the reference clocks for this device, consequently:
-	- main controller clock
-	- GOP clock
-- clock-names: names of used clocks, must be "pp_clk" and "gop_clk".
+	- one register area per Ethernet port
+  For "marvell,armda-7k-pp2", must contain the following register
+  sets:
+        - common controller registers
+	- per-port registers
+
+- clocks: pointers to the reference clocks for this device, consequently:
+	- main controller clock (for both armada-375-pp2 and armada-7k-pp2)
+	- GOP clock (for both armada-375-pp2 and armada-7k-pp2)
+	- MG clock (only for armada-7k-pp2)
+- clock-names: names of used clocks, must be "pp_clk", "gop_clk" and
+  "mg_clk" (the latter only for armada-7k-pp2).
 
 The ethernet ports are represented by subnodes. At least one port is
 required.
@@ -19,8 +30,9 @@ required.
 Required properties (port):
 
 - interrupts: interrupt for the port
-- port-id: should be '0' or '1' for ethernet ports, and '2' for the
-           loopback port
+- port-id: ID of the port from the MAC point of view
+- gop-port-id: only for marvell,armada-7k-pp2, ID of the port from the
+  GOP (Group Of Ports) point of view
 - phy-mode: See ethernet.txt file in the same directory
 
 Optional properties (port):
@@ -31,7 +43,7 @@ Optional properties (port):
   then fixed link is assumed, and the 'fixed-link' property is
   mandatory.
 
-Example:
+Example for marvell,armada-375-pp2:
 
 ethernet@f0000 {
 	compatible = "marvell,armada-375-pp2";
@@ -59,3 +71,35 @@ ethernet@f0000 {
 		phy-mode = "gmii";
 	};
 };
+
+Example for marvell,armada-7k-pp2:
+
+cpm_ethernet: ethernet@0 {
+	compatible = "marvell,armada-7k-pp22";
+	reg = <0x0 0x100000>,
+	      <0x100000 0x80000>;
+	clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>;
+	clock-names = "pp_clk", "gop_clk", "gp_clk";
+	status = "disabled";
+
+	eth0: eth@0 {
+		interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+		port-id = <0>;
+		gop-port-id = <0>;
+		status = "disabled";
+	};
+
+	eth1: eth@1 {
+		interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
+		port-id = <1>;
+		gop-port-id = <2>;
+		status = "disabled";
+	};
+
+	eth2: eth@2 {
+		interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+		port-id = <2>;
+		gop-port-id = <3>;
+		status = "disabled";
+	};
+};
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Rob Herring]

On Wed, Dec 28, 2016 at 05:46:17PM +0100, Thomas Petazzoni wrote:
> The Marvell PPv2 Device Tree binding was so far only used to describe
> the PPv2.1 network controller, used in the Marvell Armada 375.
> 
> A new version of this IP block, PPv2.2 is used in the Marvell Armada
> 7K/8K processor. This commit extends the existing binding so that it can
> also be used to describe PPv2.2 hardware.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  .../devicetree/bindings/net/marvell-pp2.txt        | 66 ++++++++++++++++++----
>  1 file changed, 55 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt
> index aa4f423..76071f3 100644
> --- a/Documentation/devicetree/bindings/net/marvell-pp2.txt
> +++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt
> @@ -1,17 +1,28 @@
> -* Marvell Armada 375 Ethernet Controller (PPv2)
> +* Marvell Armada 375 Ethernet Controller (PPv2.1)
> +  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
>  
>  Required properties:
>  
> -- compatible: should be "marvell,armada-375-pp2"
> +- compatible: should be one of:
> +    "marvell,armada-375-pp2"
> +    "marvell,armada-7k-pp2"
>  - reg: addresses and length of the register sets for the device.
> -  Must contain the following register sets:
> +  For "marvell,armada-375-pp2", must contain the following register
> +  sets:
>  	- common controller registers
>  	- LMS registers
> -  In addition, at least one port register set is required.
> -- clocks: a pointer to the reference clocks for this device, consequently:
> -	- main controller clock
> -	- GOP clock
> -- clock-names: names of used clocks, must be "pp_clk" and "gop_clk".
> +	- one register area per Ethernet port
> +  For "marvell,armda-7k-pp2", must contain the following register
> +  sets:
> +        - common controller registers
> +	- per-port registers
> +
> +- clocks: pointers to the reference clocks for this device, consequently:
> +	- main controller clock (for both armada-375-pp2 and armada-7k-pp2)
> +	- GOP clock (for both armada-375-pp2 and armada-7k-pp2)
> +	- MG clock (only for armada-7k-pp2)
> +- clock-names: names of used clocks, must be "pp_clk", "gop_clk" and
> +  "mg_clk" (the latter only for armada-7k-pp2).
>  
>  The ethernet ports are represented by subnodes. At least one port is
>  required.
> @@ -19,8 +30,9 @@ required.
>  Required properties (port):
>  
>  - interrupts: interrupt for the port
> -- port-id: should be '0' or '1' for ethernet ports, and '2' for the
> -           loopback port
> +- port-id: ID of the port from the MAC point of view
> +- gop-port-id: only for marvell,armada-7k-pp2, ID of the port from the
> +  GOP (Group Of Ports) point of view

What GOP is needs a better explanation. Why doesn't 375 need this?

>  - phy-mode: See ethernet.txt file in the same directory
>  
>  Optional properties (port):
> @@ -31,7 +43,7 @@ Optional properties (port):
>    then fixed link is assumed, and the 'fixed-link' property is
>    mandatory.
>  
> -Example:
> +Example for marvell,armada-375-pp2:
>  
>  ethernet@f0000 {
>  	compatible = "marvell,armada-375-pp2";
> @@ -59,3 +71,35 @@ ethernet@f0000 {
>  		phy-mode = "gmii";
>  	};
>  };
> +
> +Example for marvell,armada-7k-pp2:
> +
> +cpm_ethernet: ethernet@0 {
> +	compatible = "marvell,armada-7k-pp22";
> +	reg = <0x0 0x100000>,
> +	      <0x100000 0x80000>;
> +	clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>;
> +	clock-names = "pp_clk", "gop_clk", "gp_clk";
> +	status = "disabled";

Drop status from examples.

> +
> +	eth0: eth@0 {

unit address requires a reg property. Or this can be 'eth0' instead.

> +		interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
> +		port-id = <0>;
> +		gop-port-id = <0>;
> +		status = "disabled";
> +	};
> +
> +	eth1: eth@1 {
> +		interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> +		port-id = <1>;
> +		gop-port-id = <2>;
> +		status = "disabled";
> +	};
> +
> +	eth2: eth@2 {
> +		interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> +		port-id = <2>;
> +		gop-port-id = <3>;
> +		status = "disabled";
> +	};
> +};
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Thomas Petazzoni]

Hello,

On Tue, 3 Jan 2017 14:18:42 -0600, Rob Herring wrote:

> > +- port-id: ID of the port from the MAC point of view
> > +- gop-port-id: only for marvell,armada-7k-pp2, ID of the port from the
> > +  GOP (Group Of Ports) point of view  
> 
> What GOP is needs a better explanation. Why doesn't 375 need this?

GOP stands for "group of ports", it's one of the HW component inside
the PPv2 IP.

Armada 375 also has the same GOP, but we described the registers in a
different way for Armada 375, with one reg entry per port:

        reg = <0xf0000 0xa000>,
              <0xc0000 0x3060>,
              <0xc4000 0x100>,
              <0xc5000 0x100>;

The last two entries are the per-port registers for eth0 and eth1.

For PPv2.2, we wanted to simplify a little bit the register mappings,
and simply reflect the memory map of the SoC. In the SoC datasheet,
there are two memory areas for the networking subsystem, which are the
two areas reflected in:

        reg = <0x0 0x100000>,
              <0x100000 0x80000>;

The per-port registers are inside the second register area. But by
exposing the entire register area in the Device Tree binding, we allow
improvements in the driver that need additional registers to be made
without changing the Device Tree description of the device.

> > +Example for marvell,armada-7k-pp2:
> > +
> > +cpm_ethernet: ethernet@0 {
> > +	compatible = "marvell,armada-7k-pp22";
> > +	reg = <0x0 0x100000>,
> > +	      <0x100000 0x80000>;
> > +	clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>;
> > +	clock-names = "pp_clk", "gop_clk", "gp_clk";
> > +	status = "disabled";  
> 
> Drop status from examples.

OK, so I'll have to adjust this for the existing armada-375-pp2 example
as well.

> > +
> > +	eth0: eth@0 {  
> 
> unit address requires a reg property. Or this can be 'eth0' instead.

Same here, the sub-nodes don't have a reg property for armada-375-pp2,
so I guess this comment applies as well, right?

Thanks for your feedback!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Russell King - ARM Linux]

On Thu, Feb 02, 2017 at 05:56:50PM +0100, Thomas Petazzoni wrote:
> For PPv2.2, we wanted to simplify a little bit the register mappings,
> and simply reflect the memory map of the SoC. In the SoC datasheet,
> there are two memory areas for the networking subsystem, which are the
> two areas reflected in:
> 
>         reg = <0x0 0x100000>,
>               <0x100000 0x80000>;
> 
> The per-port registers are inside the second register area. But by
> exposing the entire register area in the Device Tree binding, we allow
> improvements in the driver that need additional registers to be made
> without changing the Device Tree description of the device.

Are you sure that this makes sense?  You have the serdes block at
0x120000-0x125fff, which sits within that range, and that needs to
be configured for SATA and PCIe, so it's not strictly just a network
thing.

I know from my experimentation that disabling the "serdes" by clearing
the SD_EXTERNAL_CONFIG1_REG and SD_EXTERNAL_CONFIG0_REG for SATA
channels prevents SATA working.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Thomas Petazzoni]

Hello,

On Fri, 3 Feb 2017 16:48:08 +0000, Russell King - ARM Linux wrote:
> On Thu, Feb 02, 2017 at 05:56:50PM +0100, Thomas Petazzoni wrote:
> > For PPv2.2, we wanted to simplify a little bit the register mappings,
> > and simply reflect the memory map of the SoC. In the SoC datasheet,
> > there are two memory areas for the networking subsystem, which are the
> > two areas reflected in:
> > 
> >         reg = <0x0 0x100000>,
> >               <0x100000 0x80000>;
> > 
> > The per-port registers are inside the second register area. But by
> > exposing the entire register area in the Device Tree binding, we allow
> > improvements in the driver that need additional registers to be made
> > without changing the Device Tree description of the device.  
> 
> Are you sure that this makes sense?  You have the serdes block at
> 0x120000-0x125fff, which sits within that range, and that needs to
> be configured for SATA and PCIe, so it's not strictly just a network
> thing.
> 
> I know from my experimentation that disabling the "serdes" by clearing
> the SD_EXTERNAL_CONFIG1_REG and SD_EXTERNAL_CONFIG0_REG for SATA
> channels prevents SATA working.

I have simply/stupidly followed the datasheet, which says:

 Packet processor:        0xf2000000, 0xf4000000 (i.e offset 0x0 in the CP)
 Networking interfaces:   0xf2100000, 0xf4100000 (i.e offset 0x100000 in the CP)
 IO Bridge Addr Decoding: 0xf2190000, 0xf4190000

And since there is no other block described, I assumed that the entire
range 0xf2000000 to 0xf2100000 was dedicated to the "Packet processor",
and that the range 0xf2100000 to 0xf2190000 was dedicated to the
"Networking interfaces" (and I realize the size is 0x90000, not
0x80000).

On the other hand, there are registers after 0x125fff that are really
networking related. For example, the driver is currently accessing
0x12a204, which is after the SERDES related registers.

I'll try to get some more information about this, but I suspect this is
one case where we don't yet fully understand how all the HW works and
what all registers are doing, so it's hard to do a perfect DT binding
from day 1.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Thomas Petazzoni]

Hello,

On Tue, 14 Feb 2017 15:25:03 +0100, Thomas Petazzoni wrote:

> I'll try to get some more information about this, but I suspect this is
> one case where we don't yet fully understand how all the HW works and
> what all registers are doing, so it's hard to do a perfect DT binding
> from day 1.

So I've looked into this more closely, and after drawing a diagram of
all the register areas, I came up with a much more reduced area, with
only networking related registers: starting at 0x129000 for a size of
0xb000. So now the DT node looks like this:

+                       cpm_ethernet: ethernet@0 {
+                               compatible = "marvell,armada-7k-pp22";
+                               reg = <0x0 0x100000>, <0x129000 0xb000>;

This will be part of my next iteration.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Russell King - ARM Linux]

On Wed, Dec 28, 2016 at 05:46:17PM +0100, Thomas Petazzoni wrote:
> @@ -31,7 +43,7 @@ Optional properties (port):
>    then fixed link is assumed, and the 'fixed-link' property is
>    mandatory.

Not directly related to this patch, but this context is wrong.  The
PP2 driver _requires_ a PHY.  It doesn't support fixed-link in its
current form.  I think the DT binding describes an expectation of
a future driver.

The side effect is that if trying to use a fixed-link on any port,
the ethernet driver fails to probe.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support

[Thomas Petazzoni]

Hello,

On Sat, 7 Jan 2017 09:32:28 +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 28, 2016 at 05:46:17PM +0100, Thomas Petazzoni wrote:
> > @@ -31,7 +43,7 @@ Optional properties (port):
> >    then fixed link is assumed, and the 'fixed-link' property is
> >    mandatory.  
> 
> Not directly related to this patch, but this context is wrong.  The
> PP2 driver _requires_ a PHY.  It doesn't support fixed-link in its
> current form.  I think the DT binding describes an expectation of
> a future driver.
> 
> The side effect is that if trying to use a fixed-link on any port,
> the ethernet driver fails to probe.

Correct. I will send a patch to adjust the Device Tree binding
documentation accordingly. Thanks for noticing this mistake!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCHv2 net-next 02/16] net: mvpp2: add and use accessors for TX/RX descriptors

[Thomas Petazzoni]

The PPv2.2 IP has a different TX and RX descriptor layout compared to
PPv2.1. In order to prepare for the introduction of PPv2.2 support in
mvpp2, this commit adds accessors for the different fields of the TX
and RX descriptors, and changes the code to use them.

For now, the mvpp2_port argument passed to the accessors is not used,
but it will be used in follow-up to update the descriptor according to
the version of the IP being used.

Apart from the mechanical changes to use the newly introduced
accessors, a few other changes, needed to use the accessors, are made:

 - The mvpp2_txq_inc_put() function now takes a mvpp2_port as first
   argument, as it is needed to use the accessors.

 - Similarly, the mvpp2_bm_cookie_build() gains a mvpp2_port first
   argument, for the same reason.

 - In mvpp2_rx_error(), instead of accessing the RX descriptor in each
   case of the switch, we introduce a local variable to store the
   packet size.

 - Similarly, in mvpp2_buff_hdr_rx(), we introduce a local "cookie"
   variable to store the RX descriptor cookie, rather than accessing
   it from the descriptor each time.

 - In mvpp2_tx_frag_process() and mvpp2_tx() instead of accessing the
   packet size from the TX descriptor, we use the actual value
   available in the function, which is used to set the TX descriptor
   packet size a few lines before.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/net/ethernet/marvell/mvpp2.c | 187 +++++++++++++++++++++++++----------
 1 file changed, 137 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 2268808..fd84923 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -967,6 +967,77 @@ static u32 mvpp2_read(struct mvpp2 *priv, u32 offset)
 	return readl(priv->base + offset);
 }
 
+static dma_addr_t mvpp2_txdesc_phys_addr_get(struct mvpp2_port *port,
+					     struct mvpp2_tx_desc *tx_desc)
+{
+	return tx_desc->buf_phys_addr;
+}
+
+static void mvpp2_txdesc_phys_addr_set(struct mvpp2_port *port,
+				       struct mvpp2_tx_desc *tx_desc,
+				       dma_addr_t phys_addr)
+{
+	tx_desc->buf_phys_addr = phys_addr;
+}
+
+static size_t mvpp2_txdesc_size_get(struct mvpp2_port *port,
+				    struct mvpp2_tx_desc *tx_desc)
+{
+	return tx_desc->data_size;
+}
+
+static void mvpp2_txdesc_size_set(struct mvpp2_port *port,
+				  struct mvpp2_tx_desc *tx_desc,
+				  size_t size)
+{
+	tx_desc->data_size = size;
+}
+
+static void mvpp2_txdesc_txq_set(struct mvpp2_port *port,
+				 struct mvpp2_tx_desc *tx_desc,
+				 unsigned int txq)
+{
+	tx_desc->phys_txq = txq;
+}
+
+static void mvpp2_txdesc_cmd_set(struct mvpp2_port *port,
+				 struct mvpp2_tx_desc *tx_desc,
+				 unsigned int command)
+{
+	tx_desc->command = command;
+}
+
+static void mvpp2_txdesc_offset_set(struct mvpp2_port *port,
+				    struct mvpp2_tx_desc *tx_desc,
+				    unsigned int offset)
+{
+	tx_desc->packet_offset = offset;
+}
+
+static dma_addr_t mvpp2_rxdesc_phys_addr_get(struct mvpp2_port *port,
+					     struct mvpp2_rx_desc *rx_desc)
+{
+	return rx_desc->buf_phys_addr;
+}
+
+static unsigned long mvpp2_rxdesc_virt_addr_get(struct mvpp2_port *port,
+						struct mvpp2_rx_desc *rx_desc)
+{
+	return rx_desc->buf_cookie;
+}
+
+static size_t mvpp2_rxdesc_size_get(struct mvpp2_port *port,
+				    struct mvpp2_rx_desc *rx_desc)
+{
+	return rx_desc->data_size;
+}
+
+static u32 mvpp2_rxdesc_status_get(struct mvpp2_port *port,
+				   struct mvpp2_rx_desc *rx_desc)
+{
+	return rx_desc->status;
+}
+
 static void mvpp2_txq_inc_get(struct mvpp2_txq_pcpu *txq_pcpu)
 {
 	txq_pcpu->txq_get_index++;
@@ -974,15 +1045,16 @@ static void mvpp2_txq_inc_get(struct mvpp2_txq_pcpu *txq_pcpu)
 		txq_pcpu->txq_get_index = 0;
 }
 
-static void mvpp2_txq_inc_put(struct mvpp2_txq_pcpu *txq_pcpu,
+static void mvpp2_txq_inc_put(struct mvpp2_port *port,
+			      struct mvpp2_txq_pcpu *txq_pcpu,
 			      struct sk_buff *skb,
 			      struct mvpp2_tx_desc *tx_desc)
 {
 	struct mvpp2_txq_pcpu_buf *tx_buf =
 		txq_pcpu->buffs + txq_pcpu->txq_put_index;
 	tx_buf->skb = skb;
-	tx_buf->size = tx_desc->data_size;
-	tx_buf->phys = tx_desc->buf_phys_addr;
+	tx_buf->size = mvpp2_txdesc_size_get(port, tx_desc);
+	tx_buf->phys = mvpp2_txdesc_phys_addr_get(port, tx_desc);
 	txq_pcpu->txq_put_index++;
 	if (txq_pcpu->txq_put_index == txq_pcpu->size)
 		txq_pcpu->txq_put_index = 0;
@@ -4147,11 +4219,15 @@ static void mvpp2_rxq_offset_set(struct mvpp2_port *port,
 }
 
 /* Obtain BM cookie information from descriptor */
-static u32 mvpp2_bm_cookie_build(struct mvpp2_rx_desc *rx_desc)
+static u32 mvpp2_bm_cookie_build(struct mvpp2_port *port,
+				 struct mvpp2_rx_desc *rx_desc)
 {
-	int pool = (rx_desc->status & MVPP2_RXD_BM_POOL_ID_MASK) >>
-		   MVPP2_RXD_BM_POOL_ID_OFFS;
 	int cpu = smp_processor_id();
+	int pool;
+
+	pool = (mvpp2_rxdesc_status_get(port, rx_desc) &
+		MVPP2_RXD_BM_POOL_ID_MASK) >>
+		MVPP2_RXD_BM_POOL_ID_OFFS;
 
 	return ((pool & 0xFF) << MVPP2_BM_COOKIE_POOL_OFFS) |
 	       ((cpu & 0xFF) << MVPP2_BM_COOKIE_CPU_OFFS);
@@ -4580,10 +4656,11 @@ static void mvpp2_rxq_drop_pkts(struct mvpp2_port *port,
 
 	for (i = 0; i < rx_received; i++) {
 		struct mvpp2_rx_desc *rx_desc = mvpp2_rxq_next_desc_get(rxq);
-		u32 bm = mvpp2_bm_cookie_build(rx_desc);
+		u32 bm = mvpp2_bm_cookie_build(port, rx_desc);
 
-		mvpp2_pool_refill(port, bm, rx_desc->buf_phys_addr,
-				  rx_desc->buf_cookie);
+		mvpp2_pool_refill(port, bm,
+				  mvpp2_rxdesc_phys_addr_get(port, rx_desc),
+				  mvpp2_rxdesc_virt_addr_get(port, rx_desc));
 	}
 	mvpp2_rxq_status_update(port, rxq->id, rx_received, rx_received);
 }
@@ -4972,20 +5049,21 @@ static enum hrtimer_restart mvpp2_hr_timer_cb(struct hrtimer *timer)
 static void mvpp2_rx_error(struct mvpp2_port *port,
 			   struct mvpp2_rx_desc *rx_desc)
 {
-	u32 status = rx_desc->status;
+	u32 status = mvpp2_rxdesc_status_get(port, rx_desc);
+	size_t sz = mvpp2_rxdesc_size_get(port, rx_desc);
 
 	switch (status & MVPP2_RXD_ERR_CODE_MASK) {
 	case MVPP2_RXD_ERR_CRC:
-		netdev_err(port->dev, "bad rx status %08x (crc error), size=%d\n",
-			   status, rx_desc->data_size);
+		netdev_err(port->dev, "bad rx status %08x (crc error), size=%zu\n",
+			   status, sz);
 		break;
 	case MVPP2_RXD_ERR_OVERRUN:
-		netdev_err(port->dev, "bad rx status %08x (overrun error), size=%d\n",
-			   status, rx_desc->data_size);
+		netdev_err(port->dev, "bad rx status %08x (overrun error), size=%zu\n",
+			   status, sz);
 		break;
 	case MVPP2_RXD_ERR_RESOURCE:
-		netdev_err(port->dev, "bad rx status %08x (resource error), size=%d\n",
-			   status, rx_desc->data_size);
+		netdev_err(port->dev, "bad rx status %08x (resource error), size=%zu\n",
+			   status, sz);
 		break;
 	}
 }
@@ -5061,7 +5139,7 @@ static void mvpp2_buff_hdr_rx(struct mvpp2_port *port,
 {
 	struct mvpp2_buff_hdr *buff_hdr;
 	struct sk_buff *skb;
-	u32 rx_status = rx_desc->status;
+	u32 rx_status = mvpp2_rxdesc_status_get(port, rx_desc);
 	dma_addr_t buff_phys_addr;
 	unsigned long buff_virt_addr;
 	dma_addr_t buff_phys_addr_next;
@@ -5071,8 +5149,8 @@ static void mvpp2_buff_hdr_rx(struct mvpp2_port *port,
 
 	pool_id = (rx_status & MVPP2_RXD_BM_POOL_ID_MASK) >>
 		   MVPP2_RXD_BM_POOL_ID_OFFS;
-	buff_phys_addr = rx_desc->buf_phys_addr;
-	buff_virt_addr = rx_desc->buf_cookie;
+	buff_phys_addr = mvpp2_rxdesc_phys_addr_get(port, rx_desc);
+	buff_virt_addr = mvpp2_rxdesc_virt_addr_get(port, rx_desc);
 
 	do {
 		skb = (struct sk_buff *)buff_virt_addr;
@@ -5119,12 +5197,13 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
 		void *data;
 
 		rx_done++;
-		rx_status = rx_desc->status;
-		rx_bytes = rx_desc->data_size - MVPP2_MH_SIZE;
-		phys_addr = rx_desc->buf_phys_addr;
-		data = (void *)rx_desc->buf_cookie;
+		rx_status = mvpp2_rxdesc_status_get(port, rx_desc);
+		rx_bytes = mvpp2_rxdesc_size_get(port, rx_desc);
+		rx_bytes -= MVPP2_MH_SIZE;
+		phys_addr = mvpp2_rxdesc_phys_addr_get(port, rx_desc);
+		data = (void *)mvpp2_rxdesc_virt_addr_get(port, rx_desc);
 
-		bm = mvpp2_bm_cookie_build(rx_desc);
+		bm = mvpp2_bm_cookie_build(port, rx_desc);
 		pool = mvpp2_bm_cookie_pool_get(bm);
 		bm_pool = &port->priv->bm_pools[pool];
 		/* Check if buffer header is used */
@@ -5143,9 +5222,8 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
 			dev->stats.rx_errors++;
 			mvpp2_rx_error(port, rx_desc);
 			/* Return the buffer to the pool */
-
-			mvpp2_pool_refill(port, bm, rx_desc->buf_phys_addr,
-					  rx_desc->buf_cookie);
+			mvpp2_pool_refill(port, bm, phys_addr,
+					  (unsigned long)data);
 			continue;
 		}
 
@@ -5197,11 +5275,15 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
 }
 
 static inline void
-tx_desc_unmap_put(struct device *dev, struct mvpp2_tx_queue *txq,
+tx_desc_unmap_put(struct mvpp2_port *port, struct mvpp2_tx_queue *txq,
 		  struct mvpp2_tx_desc *desc)
 {
-	dma_unmap_single(dev, desc->buf_phys_addr,
-			 desc->data_size, DMA_TO_DEVICE);
+	dma_addr_t buf_phys_addr =
+		mvpp2_txdesc_phys_addr_get(port, desc);
+	size_t buf_sz =
+		mvpp2_txdesc_size_get(port, desc);
+	dma_unmap_single(port->dev->dev.parent, buf_phys_addr,
+			 buf_sz, DMA_TO_DEVICE);
 	mvpp2_txq_desc_put(txq);
 }
 
@@ -5220,28 +5302,31 @@ static int mvpp2_tx_frag_process(struct mvpp2_port *port, struct sk_buff *skb,
 		void *addr = page_address(frag->page.p) + frag->page_offset;
 
 		tx_desc = mvpp2_txq_next_desc_get(aggr_txq);
-		tx_desc->phys_txq = txq->id;
-		tx_desc->data_size = frag->size;
+		mvpp2_txdesc_txq_set(port, tx_desc, txq->id);
+		mvpp2_txdesc_size_set(port, tx_desc, frag->size);
 
 		buf_phys_addr = dma_map_single(port->dev->dev.parent, addr,
-					       tx_desc->data_size,
+					       frag->size,
 					       DMA_TO_DEVICE);
 		if (dma_mapping_error(port->dev->dev.parent, buf_phys_addr)) {
 			mvpp2_txq_desc_put(txq);
 			goto error;
 		}
 
-		tx_desc->packet_offset = buf_phys_addr & MVPP2_TX_DESC_ALIGN;
-		tx_desc->buf_phys_addr = buf_phys_addr & (~MVPP2_TX_DESC_ALIGN);
+		mvpp2_txdesc_phys_addr_set(port, tx_desc,
+					   buf_phys_addr & MVPP2_TX_DESC_ALIGN);
+		mvpp2_txdesc_offset_set(port, tx_desc,
+					buf_phys_addr & (~MVPP2_TX_DESC_ALIGN));
 
 		if (i == (skb_shinfo(skb)->nr_frags - 1)) {
 			/* Last descriptor */
-			tx_desc->command = MVPP2_TXD_L_DESC;
-			mvpp2_txq_inc_put(txq_pcpu, skb, tx_desc);
+			mvpp2_txdesc_cmd_set(port, tx_desc,
+					     MVPP2_TXD_L_DESC);
+			mvpp2_txq_inc_put(port, txq_pcpu, skb, tx_desc);
 		} else {
 			/* Descriptor in the middle: Not First, Not Last */
-			tx_desc->command = 0;
-			mvpp2_txq_inc_put(txq_pcpu, NULL, tx_desc);
+			mvpp2_txdesc_cmd_set(port, tx_desc, 0);
+			mvpp2_txq_inc_put(port, txq_pcpu, NULL, tx_desc);
 		}
 	}
 
@@ -5253,7 +5338,7 @@ static int mvpp2_tx_frag_process(struct mvpp2_port *port, struct sk_buff *skb,
 	 */
 	for (i = i - 1; i >= 0; i--) {
 		tx_desc = txq->descs + i;
-		tx_desc_unmap_put(port->dev->dev.parent, txq, tx_desc);
+		tx_desc_unmap_put(port, txq, tx_desc);
 	}
 
 	return -ENOMEM;
@@ -5288,35 +5373,37 @@ static int mvpp2_tx(struct sk_buff *skb, struct net_device *dev)
 
 	/* Get a descriptor for the first part of the packet */
 	tx_desc = mvpp2_txq_next_desc_get(aggr_txq);
-	tx_desc->phys_txq = txq->id;
-	tx_desc->data_size = skb_headlen(skb);
+	mvpp2_txdesc_txq_set(port, tx_desc, txq->id);
+	mvpp2_txdesc_size_set(port, tx_desc, skb_headlen(skb));
 
 	buf_phys_addr = dma_map_single(dev->dev.parent, skb->data,
-				       tx_desc->data_size, DMA_TO_DEVICE);
+				       skb_headlen(skb), DMA_TO_DEVICE);
 	if (unlikely(dma_mapping_error(dev->dev.parent, buf_phys_addr))) {
 		mvpp2_txq_desc_put(txq);
 		frags = 0;
 		goto out;
 	}
-	tx_desc->packet_offset = buf_phys_addr & MVPP2_TX_DESC_ALIGN;
-	tx_desc->buf_phys_addr = buf_phys_addr & ~MVPP2_TX_DESC_ALIGN;
+	mvpp2_txdesc_offset_set(port, tx_desc,
+				buf_phys_addr & MVPP2_TX_DESC_ALIGN);
+	mvpp2_txdesc_phys_addr_set(port, tx_desc,
+				   buf_phys_addr & ~MVPP2_TX_DESC_ALIGN);
 
 	tx_cmd = mvpp2_skb_tx_csum(port, skb);
 
 	if (frags == 1) {
 		/* First and Last descriptor */
 		tx_cmd |= MVPP2_TXD_F_DESC | MVPP2_TXD_L_DESC;
-		tx_desc->command = tx_cmd;
-		mvpp2_txq_inc_put(txq_pcpu, skb, tx_desc);
+		mvpp2_txdesc_cmd_set(port, tx_desc, tx_cmd);
+		mvpp2_txq_inc_put(port, txq_pcpu, skb, tx_desc);
 	} else {
 		/* First but not Last */
 		tx_cmd |= MVPP2_TXD_F_DESC | MVPP2_TXD_PADDING_DISABLE;
-		tx_desc->command = tx_cmd;
-		mvpp2_txq_inc_put(txq_pcpu, NULL, tx_desc);
+		mvpp2_txdesc_cmd_set(port, tx_desc, tx_cmd);
+		mvpp2_txq_inc_put(port, txq_pcpu, NULL, tx_desc);
 
 		/* Continue with other skb fragments */
 		if (mvpp2_tx_frag_process(port, skb, aggr_txq, txq)) {
-			tx_desc_unmap_put(port->dev->dev.parent, txq, tx_desc);
+			tx_desc_unmap_put(port, txq, tx_desc);
 			frags = 0;
 			goto out;
 		}
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 net-next 03/16] net: mvpp2: add hw_version field in "struct mvpp2"

[Thomas Petazzoni]

In preparation to the introduction for the support of PPv2.2 in the
mvpp2 driver, this commit adds a hw_version field to the struct
mvpp2, and uses the .data field of the DT match table to fill it in.

Having the MVPP21 and MVPP22 definitions available will allow to start
adding the necessary conditional code to support PPv2.2.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/net/ethernet/marvell/mvpp2.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index fd84923..bc359a9 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -25,6 +25,7 @@
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/phy.h>
 #include <linux/clk.h>
 #include <linux/hrtimer.h>
@@ -649,6 +650,9 @@ struct mvpp2 {
 
 	/* Tclk value */
 	u32 tclk;
+
+	/* HW version */
+	enum { MVPP21, MVPP22 } hw_version;
 };
 
 struct mvpp2_pcpu_stats {
@@ -6480,6 +6484,9 @@ static int mvpp2_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->hw_version =
+		(unsigned long)of_device_get_match_data(&pdev->dev);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	priv->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(priv->base))
@@ -6584,7 +6591,10 @@ static int mvpp2_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id mvpp2_match[] = {
-	{ .compatible = "marvell,armada-375-pp2" },
+	{
+		.compatible = "marvell,armada-375-pp2",
+		.data = (void *)MVPP21,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mvpp2_match);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 net-next 07/16] net: mvpp2: adapt the mvpp2_rxq_*_pool_set functions to PPv2.2

[Thomas Petazzoni]

The MVPP2_RXQ_CONFIG_REG register has a slightly different layout
between PPv2.1 and PPv2.2, so this commit adapts the functions modifying
this register to accommodate for both the PPv2.1 and PPv2.2 cases.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/net/ethernet/marvell/mvpp2.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 160b787..8fc818d 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -50,9 +50,11 @@
 #define     MVPP2_SNOOP_PKT_SIZE_MASK		0x1ff
 #define     MVPP2_SNOOP_BUF_HDR_MASK		BIT(9)
 #define     MVPP2_RXQ_POOL_SHORT_OFFS		20
-#define     MVPP2_RXQ_POOL_SHORT_MASK		0x700000
+#define     MVPP21_RXQ_POOL_SHORT_MASK		0x700000
+#define     MVPP22_RXQ_POOL_SHORT_MASK		0xf00000
 #define     MVPP2_RXQ_POOL_LONG_OFFS		24
-#define     MVPP2_RXQ_POOL_LONG_MASK		0x7000000
+#define     MVPP21_RXQ_POOL_LONG_MASK		0x7000000
+#define     MVPP22_RXQ_POOL_LONG_MASK		0xf000000
 #define     MVPP2_RXQ_PACKET_OFFSET_OFFS	28
 #define     MVPP2_RXQ_PACKET_OFFSET_MASK	0x70000000
 #define     MVPP2_RXQ_DISABLE_MASK		BIT(31)
@@ -3762,17 +3764,20 @@ static int mvpp2_bm_init(struct platform_device *pdev, struct mvpp2 *priv)
 static void mvpp2_rxq_long_pool_set(struct mvpp2_port *port,
 				    int lrxq, int long_pool)
 {
-	u32 val;
+	u32 val, mask;
 	int prxq;
 
 	/* Get queue physical ID */
 	prxq = port->rxqs[lrxq]->id;
 
-	val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(prxq));
-	val &= ~MVPP2_RXQ_POOL_LONG_MASK;
-	val |= ((long_pool << MVPP2_RXQ_POOL_LONG_OFFS) &
-		    MVPP2_RXQ_POOL_LONG_MASK);
+	if (port->priv->hw_version == MVPP21)
+		mask = MVPP21_RXQ_POOL_LONG_MASK;
+	else
+		mask = MVPP22_RXQ_POOL_LONG_MASK;
 
+	val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(prxq));
+	val &= ~mask;
+	val |= (long_pool << MVPP2_RXQ_POOL_LONG_OFFS) & mask;
 	mvpp2_write(port->priv, MVPP2_RXQ_CONFIG_REG(prxq), val);
 }
 
@@ -3780,17 +3785,20 @@ static void mvpp2_rxq_long_pool_set(struct mvpp2_port *port,
 static void mvpp2_rxq_short_pool_set(struct mvpp2_port *port,
 				     int lrxq, int short_pool)
 {
-	u32 val;
+	u32 val, mask;
 	int prxq;
 
 	/* Get queue physical ID */
 	prxq = port->rxqs[lrxq]->id;
 
-	val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(prxq));
-	val &= ~MVPP2_RXQ_POOL_SHORT_MASK;
-	val |= ((short_pool << MVPP2_RXQ_POOL_SHORT_OFFS) &
-		    MVPP2_RXQ_POOL_SHORT_MASK);
+	if (port->priv->hw_version == MVPP21)
+		mask = MVPP21_RXQ_POOL_SHORT_MASK;
+	else
+		mask = MVPP22_RXQ_POOL_SHORT_MASK;
 
+	val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(prxq));
+	val &= ~mask;
+	val |= (short_pool << MVPP2_RXQ_POOL_SHORT_OFFS) & mask;
 	mvpp2_write(port->priv, MVPP2_RXQ_CONFIG_REG(prxq), val);
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 net-next 14/16] net: mvpp2: adapt rxq distribution to PPv2.2

[Thomas Petazzoni]

In PPv2.1, we have a maximum of 8 RXQs per port, with a default of 4
RXQs per port, and we were assigning RXQs 0->3 to the first port, 4->7
to the second port, 8->11 to the third port, etc.

In PPv2.2, we have a maximum of 32 RXQs per port, and we must allocate
RXQs from the range of 32 RXQs available for each port. So port 0 must
use RXQs in the range 0->31, port 1 in the range 32->63, etc.

This commit adapts the mvpp2 to this difference between PPv2.1 and
PPv2.2:

 - The constant definition MVPP2_MAX_RXQ is replaced by a new field
   'max_port_rxqs' in 'struct mvpp2', which stores the maximum number of
   RXQs per port. This field is initialized during ->probe() depending
   on the IP version.

 - MVPP2_RXQ_TOTAL_NUM is removed, and instead we calculate the total
   number of RXQs by multiplying the number of ports by the maximum of
   RXQs per port. This was anyway used in only one place.

 - In mvpp2_port_probe(), the calculation of port->first_rxq is adjusted
   to cope with the different allocation strategy between PPv2.1 and
   PPv2.2. Due to this change, the 'next_first_rxq' argument of this
   function is no longer needed and is removed.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/net/ethernet/marvell/mvpp2.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index baad991..20e9429 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -399,15 +399,9 @@
 /* Maximum number of TXQs used by single port */
 #define MVPP2_MAX_TXQ			8
 
-/* Maximum number of RXQs used by single port */
-#define MVPP2_MAX_RXQ			8
-
 /* Dfault number of RXQs in use */
 #define MVPP2_DEFAULT_RXQ		4
 
-/* Total number of RXQs available to all ports */
-#define MVPP2_RXQ_TOTAL_NUM		(MVPP2_MAX_PORTS * MVPP2_MAX_RXQ)
-
 /* Max number of Rx descriptors */
 #define MVPP2_MAX_RXD			128
 
@@ -728,6 +722,9 @@ struct mvpp2 {
 
 	/* HW version */
 	enum { MVPP21, MVPP22 } hw_version;
+
+	/* Maximum number of RXQs per port */
+	unsigned int max_port_rxqs;
 };
 
 struct mvpp2_pcpu_stats {
@@ -6333,7 +6330,8 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 	struct mvpp2_txq_pcpu *txq_pcpu;
 	int queue, cpu, err;
 
-	if (port->first_rxq + rxq_number > MVPP2_RXQ_TOTAL_NUM)
+	if (port->first_rxq + rxq_number >
+	    MVPP2_MAX_PORTS * priv->max_port_rxqs)
 		return -EINVAL;
 
 	/* Disable port */
@@ -6452,8 +6450,7 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 /* Ports initialization */
 static int mvpp2_port_probe(struct platform_device *pdev,
 			    struct device_node *port_node,
-			    struct mvpp2 *priv,
-			    int *next_first_rxq)
+			    struct mvpp2 *priv)
 {
 	struct device_node *phy_node;
 	struct mvpp2_port *port;
@@ -6511,7 +6508,11 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 
 	port->priv = priv;
 	port->id = id;
-	port->first_rxq = *next_first_rxq;
+	if (priv->hw_version == MVPP21)
+		port->first_rxq = port->id * rxq_number;
+	else
+		port->first_rxq = port->id * priv->max_port_rxqs;
+
 	port->phy_node = phy_node;
 	port->phy_interface = phy_mode;
 
@@ -6608,8 +6609,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	}
 	netdev_info(dev, "Using %s mac address %pM\n", mac_from, dev->dev_addr);
 
-	/* Increment the first Rx queue number to be used by the next port */
-	*next_first_rxq += rxq_number;
 	priv->port_list[id] = port;
 	return 0;
 
@@ -6755,7 +6754,7 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	u32 val;
 
 	/* Checks for hardware constraints */
-	if (rxq_number % 4 || (rxq_number > MVPP2_MAX_RXQ) ||
+	if (rxq_number % 4 || (rxq_number > priv->max_port_rxqs) ||
 	    (txq_number > MVPP2_MAX_TXQ)) {
 		dev_err(&pdev->dev, "invalid queue size parameter\n");
 		return -EINVAL;
@@ -6844,7 +6843,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 	struct device_node *port_node;
 	struct mvpp2 *priv;
 	struct resource *res;
-	int port_count, first_rxq, cpu;
+	int port_count, cpu;
 	int err;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct mvpp2), GFP_KERNEL);
@@ -6879,6 +6878,11 @@ static int mvpp2_probe(struct platform_device *pdev)
 		priv->cpu_base[cpu] = priv->base + cpu * addr_space_sz;
 	}
 
+	if (priv->hw_version == MVPP21)
+		priv->max_port_rxqs = 8;
+	else
+		priv->max_port_rxqs = 32;
+
 	priv->pp_clk = devm_clk_get(&pdev->dev, "pp_clk");
 	if (IS_ERR(priv->pp_clk))
 		return PTR_ERR(priv->pp_clk);
@@ -6921,9 +6925,8 @@ static int mvpp2_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize ports */
-	first_rxq = 0;
 	for_each_available_child_of_node(dn, port_node) {
-		err = mvpp2_port_probe(pdev, port_node, priv, &first_rxq);
+		err = mvpp2_port_probe(pdev, port_node, priv);
 		if (err < 0)
 			goto err_gop_clk;
 	}
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 net-next 16/16] net: mvpp2: finally add the PPv2.2 compatible string

[Thomas Petazzoni]

Now that the mvpp2 driver has been modified to accommodate the support
for PPv2.2, we can finally advertise this support by adding the
appropriate compatible string.

At the same time, we update the Kconfig description of the MVPP2 driver.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/net/ethernet/marvell/Kconfig | 4 ++--
 drivers/net/ethernet/marvell/mvpp2.c | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index d2555e8b..da6fb82 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -82,13 +82,13 @@ config MVNETA_BM
 	  that all dependencies are met.
 
 config MVPP2
-	tristate "Marvell Armada 375 network interface support"
+	tristate "Marvell Armada 375/7K/8K network interface support"
 	depends on ARCH_MVEBU || COMPILE_TEST
 	depends on HAS_DMA
 	select MVMDIO
 	---help---
 	  This driver supports the network interface units in the
-	  Marvell ARMADA 375 SoC.
+	  Marvell ARMADA 375, 7K and 8K SoCs.
 
 config PXA168_ETH
 	tristate "Marvell pxa168 ethernet support"
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 194de00..9e744d2 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -6997,6 +6997,10 @@ static const struct of_device_id mvpp2_match[] = {
 		.compatible = "marvell,armada-375-pp2",
 		.data = (void *)MVPP21,
 	},
+	{
+		.compatible = "marvell,armada-7k-pp22",
+		.data = (void *)MVPP22,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mvpp2_match);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 00/16] net: mvpp2: add basic support for PPv2.2

[David Miller]

From: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Date: Wed, 28 Dec 2016 17:46:16 +0100

> This series depends on the series named "net: mvpp2: misc improvements
> and preparation patches".

Please in the future only submit one patch series at a time.

If I've told you that a large patch series is hard to review and that
therefore one should keep each submitted series small and to a
reasonable size, that is completely undermined when you submit
multiple series to work around that request.

Thank you.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 00/16] net: mvpp2: add basic support for PPv2.2

[Thomas Petazzoni]

Hello,

On Wed, 28 Dec 2016 12:06:44 -0500 (EST), David Miller wrote:

> > This series depends on the series named "net: mvpp2: misc improvements
> > and preparation patches".  
> 
> Please in the future only submit one patch series at a time.
> 
> If I've told you that a large patch series is hard to review and that
> therefore one should keep each submitted series small and to a
> reasonable size, that is completely undermined when you submit
> multiple series to work around that request.

Sure. I'll wait for the first patch series to be merged (potentially
after several iterations) before resending the second patch series.

Thanks for the feedback!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 net-next 08/16] net: mvpp2: adapt mvpp2_defaults_set() to PPv2.2

[Thomas Petazzoni]

This commit modifies the mvpp2_defaults_set() function to not do the
loopback and FIFO threshold initialization, which are not needed for
PPv2.2.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 8fc818d..23f2368 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -4205,16 +4205,18 @@ static void mvpp2_defaults_set(struct mvpp2_port *port)
 {
 	int tx_port_num, val, queue, ptxq, lrxq;
 
-	/* Configure port to loopback if needed */
-	if (port->flags & MVPP2_F_LOOPBACK)
-		mvpp2_port_loopback_set(port);
-
-	/* Update TX FIFO MIN Threshold */
-	val = readl(port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
-	val &= ~MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK;
-	/* Min. TX threshold must be less than minimal packet length */
-	val |= MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(64 - 4 - 2);
-	writel(val, port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
+	if (port->priv->hw_version == MVPP21) {
+		/* Configure port to loopback if needed */
+		if (port->flags & MVPP2_F_LOOPBACK)
+			mvpp2_port_loopback_set(port);
+
+		/* Update TX FIFO MIN Threshold */
+		val = readl(port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
+		val &= ~MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK;
+		/* Min. TX threshold must be less than minimal packet length */
+		val |= MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(64 - 4 - 2);
+		writel(val, port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
+	}
 
 	/* Disable Legacy WRR, Disable EJP, Release from reset */
 	tx_port_num = mvpp2_egress_port(port);
-- 
2.7.4

[PATCHv2 net-next 09/16] net: mvpp2: adjust mvpp2_{rxq,txq}_init for PPv2.2

[Thomas Petazzoni]

In PPv2.2, the MVPP2_RXQ_DESC_ADDR_REG and MVPP2_TXQ_DESC_ADDR_REG
registers have a slightly different layout, because they need to contain
a 64-bit address for the RX and TX descriptor arrays. This commit
adjusts those functions accordingly.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 23f2368..22f7970 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -102,6 +102,7 @@
 /* Descriptor Manager Top Registers */
 #define MVPP2_RXQ_NUM_REG			0x2040
 #define MVPP2_RXQ_DESC_ADDR_REG			0x2044
+#define     MVPP22_DESC_ADDR_OFFS		8
 #define MVPP2_RXQ_DESC_SIZE_REG			0x2048
 #define     MVPP2_RXQ_DESC_SIZE_MASK		0x3ff0
 #define MVPP2_RXQ_STATUS_UPDATE_REG(rxq)	(0x3000 + 4 * (rxq))
@@ -143,6 +144,7 @@
 #define MVPP2_TXQ_RSVD_CLR_REG			0x20b8
 #define     MVPP2_TXQ_RSVD_CLR_OFFSET		16
 #define MVPP2_AGGR_TXQ_DESC_ADDR_REG(cpu)	(0x2100 + 4 * (cpu))
+#define     MVPP22_AGGR_TXQ_DESC_ADDR_OFFS	8
 #define MVPP2_AGGR_TXQ_DESC_SIZE_REG(cpu)	(0x2140 + 4 * (cpu))
 #define     MVPP2_AGGR_TXQ_DESC_SIZE_MASK	0x3ff0
 #define MVPP2_AGGR_TXQ_STATUS_REG(cpu)		(0x2180 + 4 * (cpu))
@@ -4769,6 +4771,8 @@ static int mvpp2_aggr_txq_init(struct platform_device *pdev,
 			       int desc_num, int cpu,
 			       struct mvpp2 *priv)
 {
+	u32 txq_phys;
+
 	/* Allocate memory for TX descriptors */
 	aggr_txq->descs = dma_alloc_coherent(&pdev->dev,
 				desc_num * MVPP2_DESC_ALIGNED_SIZE,
@@ -4782,10 +4786,16 @@ static int mvpp2_aggr_txq_init(struct platform_device *pdev,
 	aggr_txq->next_desc_to_proc = mvpp2_read(priv,
 						 MVPP2_AGGR_TXQ_INDEX_REG(cpu));
 
-	/* Set Tx descriptors queue starting address */
-	/* indirect access */
-	mvpp2_write(priv, MVPP2_AGGR_TXQ_DESC_ADDR_REG(cpu),
-		    aggr_txq->descs_phys);
+	/* Set Tx descriptors queue starting address indirect
+	 * access
+	 */
+	if (priv->hw_version == MVPP21)
+		txq_phys = aggr_txq->descs_phys;
+	else
+		txq_phys = aggr_txq->descs_phys >>
+			MVPP22_AGGR_TXQ_DESC_ADDR_OFFS;
+
+	mvpp2_write(priv, MVPP2_AGGR_TXQ_DESC_ADDR_REG(cpu), txq_phys);
 	mvpp2_write(priv, MVPP2_AGGR_TXQ_DESC_SIZE_REG(cpu), desc_num);
 
 	return 0;
@@ -4796,6 +4806,8 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
 			  struct mvpp2_rx_queue *rxq)
 
 {
+	u32 rxq_phys;
+
 	rxq->size = port->rx_ring_size;
 
 	/* Allocate memory for RX descriptors */
@@ -4812,7 +4824,11 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
 
 	/* Set Rx descriptors queue starting address - indirect access */
 	mvpp2_write(port->priv, MVPP2_RXQ_NUM_REG, rxq->id);
-	mvpp2_write(port->priv, MVPP2_RXQ_DESC_ADDR_REG, rxq->descs_phys);
+	if (port->priv->hw_version == MVPP21)
+		rxq_phys = rxq->descs_phys;
+	else
+		rxq_phys = rxq->descs_phys >> MVPP22_DESC_ADDR_OFFS;
+	mvpp2_write(port->priv, MVPP2_RXQ_DESC_ADDR_REG, rxq_phys);
 	mvpp2_write(port->priv, MVPP2_RXQ_DESC_SIZE_REG, rxq->size);
 	mvpp2_write(port->priv, MVPP2_RXQ_INDEX_REG, 0);
 
-- 
2.7.4

[PATCHv2 net-next 10/16] net: mvpp2: handle register mapping and access for PPv2.2

[Thomas Petazzoni]

This commit adjusts the mvpp2 driver register mapping and access logic
to support PPv2.2, to handle a number of differences.

Due to how the registers are laid out in memory, the Device Tree binding
for the "reg" property is different:

 - On PPv2.1, we had a first area for the common registers, and then one
   area per port.

 - On PPv2.2, we have a first area for the common registers, and a
   second area for all the per-ports registers.

In addition, on PPv2.2, the area for the common registers is split into
so-called "address spaces" of 64 KB each. They allow to access the same
registers, but from different CPUs. Hence the introduction of cpu_base[]
in 'struct mvpp2', and the modification of the mvpp2_write() and
mvpp2_read() register accessors. For PPv2.1, the compatibility is
preserved by using an "address space" size of 0.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 78 +++++++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 22f7970..389cc62 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -304,6 +304,9 @@
 #define      MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(v)	(((v) << 6) & \
 					MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK)
 
+#define MVPP22_PORT_BASE			0x30e00
+#define MVPP22_PORT_OFFSET			0x1000
+
 #define MVPP2_CAUSE_TXQ_SENT_DESC_ALL_MASK	0xff
 
 /* Descriptor ring Macros */
@@ -631,6 +634,11 @@ enum mvpp2_prs_l3_cast {
  */
 #define MVPP2_BM_SHORT_PKT_SIZE		MVPP2_RX_MAX_PKT_SIZE(512)
 
+#define MVPP21_ADDR_SPACE_SZ		0
+#define MVPP22_ADDR_SPACE_SZ		SZ_64K
+
+#define MVPP2_MAX_CPUS			4
+
 enum mvpp2_bm_type {
 	MVPP2_BM_FREE,
 	MVPP2_BM_SWF_LONG,
@@ -644,6 +652,13 @@ struct mvpp2 {
 	/* Shared registers' base addresses */
 	void __iomem *base;
 	void __iomem *lms_base;
+	void __iomem *iface_base;
+
+	/* On PPv2.2, each CPU can access the base register through a
+	 * separate address space, each 64 KB apart from each
+	 * other.
+	 */
+	void __iomem *cpu_base[MVPP2_MAX_CPUS];
 
 	/* Common clocks */
 	struct clk *pp_clk;
@@ -1021,12 +1036,21 @@ static int txq_number = MVPP2_MAX_TXQ;
 
 static void mvpp2_write(struct mvpp2 *priv, u32 offset, u32 data)
 {
-	writel(data, priv->base + offset);
+	int cpu = get_cpu();
+
+	writel(data, priv->cpu_base[cpu] + offset);
+	put_cpu();
 }
 
 static u32 mvpp2_read(struct mvpp2 *priv, u32 offset)
 {
-	return readl(priv->base + offset);
+	int cpu = get_cpu();
+	u32 val;
+
+	val = readl(priv->cpu_base[cpu] + offset);
+	put_cpu();
+
+	return val;
 }
 
 static dma_addr_t mvpp2_txdesc_phys_addr_get(struct mvpp2_port *port,
@@ -6386,7 +6410,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	u32 id;
 	int features;
 	int phy_mode;
-	int priv_common_regs_num = 2;
 	int err, i, cpu;
 
 	dev = alloc_etherdev_mqs(sizeof(struct mvpp2_port), txq_number,
@@ -6436,12 +6459,24 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	port->phy_node = phy_node;
 	port->phy_interface = phy_mode;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM,
-				    priv_common_regs_num + id);
-	port->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(port->base)) {
-		err = PTR_ERR(port->base);
-		goto err_free_irq;
+	if (priv->hw_version == MVPP21) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 2 + id);
+		port->base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(port->base)) {
+			err = PTR_ERR(port->base);
+			goto err_free_irq;
+		}
+	} else {
+		u32 gop_id;
+
+		if (of_property_read_u32(port_node, "gop-port-id", &gop_id)) {
+			err = -EINVAL;
+			dev_err(&pdev->dev, "missing gop-port-id value\n");
+			goto err_free_irq;
+		}
+
+		port->base = priv->iface_base + MVPP22_PORT_BASE +
+			gop_id * MVPP22_PORT_OFFSET;
 	}
 
 	/* Alloc per-cpu stats */
@@ -6674,7 +6709,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 	struct device_node *port_node;
 	struct mvpp2 *priv;
 	struct resource *res;
-	int port_count, first_rxq;
+	int port_count, first_rxq, cpu;
 	int err;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct mvpp2), GFP_KERNEL);
@@ -6689,10 +6724,25 @@ static int mvpp2_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	priv->lms_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(priv->lms_base))
-		return PTR_ERR(priv->lms_base);
+	if (priv->hw_version == MVPP21) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		priv->lms_base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->lms_base))
+			return PTR_ERR(priv->lms_base);
+	} else {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		priv->iface_base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->iface_base))
+			return PTR_ERR(priv->iface_base);
+	}
+
+	for_each_present_cpu(cpu) {
+		u32 addr_space_sz;
+
+		addr_space_sz = (priv->hw_version == MVPP21 ?
+				 MVPP21_ADDR_SPACE_SZ : MVPP22_ADDR_SPACE_SZ);
+		priv->cpu_base[cpu] = priv->base + cpu * addr_space_sz;
+	}
 
 	priv->pp_clk = devm_clk_get(&pdev->dev, "pp_clk");
 	if (IS_ERR(priv->pp_clk))
-- 
2.7.4

Re: [PATCHv2 net-next 10/16] net: mvpp2: handle register mapping and access for PPv2.2

[Russell King - ARM Linux]

On Wed, Dec 28, 2016 at 05:46:26PM +0100, Thomas Petazzoni wrote:
> This commit adjusts the mvpp2 driver register mapping and access logic
> to support PPv2.2, to handle a number of differences.
> 
> Due to how the registers are laid out in memory, the Device Tree binding
> for the "reg" property is different:
> 
>  - On PPv2.1, we had a first area for the common registers, and then one
>    area per port.
> 
>  - On PPv2.2, we have a first area for the common registers, and a
>    second area for all the per-ports registers.
> 
> In addition, on PPv2.2, the area for the common registers is split into
> so-called "address spaces" of 64 KB each. They allow to access the same
> registers, but from different CPUs. Hence the introduction of cpu_base[]
> in 'struct mvpp2', and the modification of the mvpp2_write() and
> mvpp2_read() register accessors. For PPv2.1, the compatibility is
> preserved by using an "address space" size of 0.

I'm not entirely sure this is the best solution - every register access
will be wrapped with a preempt_disable() and preempt_enable().  At
every site, when preempt is enabled, we will end up with code to:

- get the thread info
- increment the preempt count
- access the register
- decrement the preempt count
- test resulting preempt count and branch to __preempt_schedule()

If tracing is enabled, it gets much worse, because the increment and
decrement happen out of line, and are even more expensive.

If a function is going to make several register accesses, it's going
to be much more efficient to do:

	void __iomem *base = priv->cpu_base[get_cpu()];

	...

	put_cpu();

which means we don't end up with multiple instances of the preempt code
consecutive accesses.

I think this is an example where having driver-private accessors for
readl()/writel() is far from a good idea.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 net-next 10/16] net: mvpp2: handle register mapping and access for PPv2.2

[Thomas Petazzoni]

Hello,

On Fri, 6 Jan 2017 14:46:48 +0000, Russell King - ARM Linux wrote:

> I'm not entirely sure this is the best solution - every register access
> will be wrapped with a preempt_disable() and preempt_enable().  At
> every site, when preempt is enabled, we will end up with code to:
> 
> - get the thread info
> - increment the preempt count
> - access the register
> - decrement the preempt count
> - test resulting preempt count and branch to __preempt_schedule()
> 
> If tracing is enabled, it gets much worse, because the increment and
> decrement happen out of line, and are even more expensive.
> 
> If a function is going to make several register accesses, it's going
> to be much more efficient to do:
> 
> 	void __iomem *base = priv->cpu_base[get_cpu()];
> 
> 	...
> 
> 	put_cpu();
> 
> which means we don't end up with multiple instances of the preempt code
> consecutive accesses.

In the next version, these accessors have been reworked. After getting
more details about the HW, it turned out that disabling preemption is
not at all necessary. We just have some global registers (can be
accessed through any per-CPU window) and some per-CPU registers (must
be accessed from a specific per-CPU window), with the trick that
certain sequences of register read/write must occur from the same
per-CPU window.

So we'll have mvpp2_{read,write} that read/write through the register
window of CPU0, used for all non-per CPU register accesses. And we'll
have mvpp2_percpu_{read,write}, taking an additional "cpu" argument,
used for per-CPU register accesses. Thanks to "cpu" being passed as
argument, the caller can ensure the same cpu window is used to do
several reads/writes in a row.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences

[Thomas Petazzoni]

This commit handles a few miscellaneous differences between PPv2.1 and
PPv2.2 in different areas, where code done for PPv2.1 doesn't apply for
PPv2.2 or needs to be adjusted (getting the MAC address, disabling PHY
polling, etc.).

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 389cc62..eb55576 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -304,6 +304,9 @@
 #define      MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(v)	(((v) << 6) & \
 					MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK)
 
+#define MVPP22_SMI_MISC_CFG_REG			0x2a204
+#define      MVPP22_SMI_POLLING_EN		BIT(10)
+
 #define MVPP22_PORT_BASE			0x30e00
 #define MVPP22_PORT_OFFSET			0x1000
 
@@ -5823,7 +5826,7 @@ static int mvpp2_check_ringparam_valid(struct net_device *dev,
 	return 0;
 }
 
-static void mvpp2_get_mac_address(struct mvpp2_port *port, unsigned char *addr)
+static void mvpp21_get_mac_address(struct mvpp2_port *port, unsigned char *addr)
 {
 	u32 mac_addr_l, mac_addr_m, mac_addr_h;
 
@@ -6272,7 +6275,7 @@ static const struct ethtool_ops mvpp2_eth_tool_ops = {
 
 /* Driver initialization */
 
-static void mvpp2_port_power_up(struct mvpp2_port *port)
+static void mvpp21_port_power_up(struct mvpp2_port *port)
 {
 	mvpp2_port_mii_set(port);
 	mvpp2_port_periodic_xon_disable(port);
@@ -6491,7 +6494,8 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		mac_from = "device tree";
 		ether_addr_copy(dev->dev_addr, dt_mac_addr);
 	} else {
-		mvpp2_get_mac_address(port, hw_mac_addr);
+		if (priv->hw_version == MVPP21)
+			mvpp21_get_mac_address(port, hw_mac_addr);
 		if (is_valid_ether_addr(hw_mac_addr)) {
 			mac_from = "hardware";
 			ether_addr_copy(dev->dev_addr, hw_mac_addr);
@@ -6511,7 +6515,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		dev_err(&pdev->dev, "failed to init port %d\n", id);
 		goto err_free_stats;
 	}
-	mvpp2_port_power_up(port);
+
+	if (priv->hw_version == MVPP21)
+		mvpp21_port_power_up(port);
 
 	port->pcpu = alloc_percpu(struct mvpp2_port_pcpu);
 	if (!port->pcpu) {
@@ -6654,9 +6660,15 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 		mvpp2_conf_mbus_windows(dram_target_info, priv);
 
 	/* Disable HW PHY polling */
-	val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
-	val |= MVPP2_PHY_AN_STOP_SMI0_MASK;
-	writel(val, priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
+	if (priv->hw_version == MVPP21) {
+		val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
+		val |= MVPP2_PHY_AN_STOP_SMI0_MASK;
+		writel(val, priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
+	} else {
+		val = readl(priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
+		val &= ~MVPP22_SMI_POLLING_EN;
+		writel(val, priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
+	}
 
 	/* Allocate and initialize aggregated TXQs */
 	priv->aggr_txqs = devm_kcalloc(&pdev->dev, num_present_cpus(),
@@ -6681,8 +6693,9 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	for (i = 0; i < MVPP2_MAX_PORTS; i++)
 		mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(i), rxq_number);
 
-	writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT,
-	       priv->lms_base + MVPP2_MNG_EXTENDED_GLOBAL_CTRL_REG);
+	if (priv->hw_version == MVPP21)
+		writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT,
+		       priv->lms_base + MVPP2_MNG_EXTENDED_GLOBAL_CTRL_REG);
 
 	/* Allow cache snoop when transmiting packets */
 	mvpp2_write(priv, MVPP2_TX_SNOOP_REG, 0x1);
-- 
2.7.4

Re: [PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences

[Russell King - ARM Linux]

On Wed, Dec 28, 2016 at 05:46:27PM +0100, Thomas Petazzoni wrote:
> @@ -6511,7 +6515,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
>  		dev_err(&pdev->dev, "failed to init port %d\n", id);
>  		goto err_free_stats;
>  	}
> -	mvpp2_port_power_up(port);
> +
> +	if (priv->hw_version == MVPP21)
> +		mvpp21_port_power_up(port);

This has the side effect that nothing clears the port reset bit in the
GMAC, which means there's no hope of the interface working - with the
reset bit set, the port is well and truely held in "link down" state.

In any case, the GMAC part is much the same as mvneta, and I think
that code should be shared rather than writing new versions of it.
There are some subtle differences between neta, pp2.1 and pp2.2, but
it's entirely doable (I have an implementation here as I wasn't going
to duplicate this code for my phylink conversion.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences

[Russell King - ARM Linux]

On Sat, Jan 07, 2017 at 09:38:34AM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 28, 2016 at 05:46:27PM +0100, Thomas Petazzoni wrote:
> > @@ -6511,7 +6515,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> >  		dev_err(&pdev->dev, "failed to init port %d\n", id);
> >  		goto err_free_stats;
> >  	}
> > -	mvpp2_port_power_up(port);
> > +
> > +	if (priv->hw_version == MVPP21)
> > +		mvpp21_port_power_up(port);
> 
> This has the side effect that nothing clears the port reset bit in the
> GMAC, which means there's no hope of the interface working - with the
> reset bit set, the port is well and truely held in "link down" state.
> 
> In any case, the GMAC part is much the same as mvneta, and I think
> that code should be shared rather than writing new versions of it.
> There are some subtle differences between neta, pp2.1 and pp2.2, but
> it's entirely doable (I have an implementation here as I wasn't going
> to duplicate this code for my phylink conversion.)

In addition to comphy configuration and the above, I also need the
following to have working SGMII.  The change of MACMODE is needed
because uboot has configured the port for 10Gbase-R mode (it has a
10G PHY on it, but the PHY switches to SGMII in <10G modes.)  The
GMAC control register 4 is needed to properly configure for SGMII
mode.  I also included RGMII mode as well in there, as I expect you'd
need it to have GMAC properly configured for RGMII.

With this in place (and the other bits mentioned above), I can ping
the clearfog switch on the other end of eth0's cable:

# ping6 -I eth0 fe80::250:43ff:fe02:302
PING fe80::250:43ff:fe02:302(fe80::250:43ff:fe02:302) from fe80::200:ff:fe00:1 eth0: 56 data bytes
64 bytes from fe80::250:43ff:fe02:302: icmp_seq=1 ttl=64 time=0.297 ms

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index bc97eebf7eee..4b6ec6213e9c 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -345,7 +345,17 @@
 #define      MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK	0x1fc0
 #define      MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(v)	(((v) << 6) & \
 					MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK)
+#define MVPP22_GMAC_CTRL_4_REG			0x90
+#define      MVPP22_CTRL4_EXT_PIN_GMII_SEL	BIT(0)
+#define      MVPP22_CTRL4_DP_CLK_SEL		BIT(5)
+#define      MVPP22_CTRL4_SYNC_BYPASS		BIT(6)
+#define      MVPP22_CTRL4_QSGMII_BYPASS_ACTIVE	BIT(7)
+
+#define MVPP22_XLG_CTRL3_REG			0x11c
+#define      MVPP22_XLG_CTRL3_MACMODESELECT_MASK	(7 << 13)
+#define      MVPP22_XLG_CTRL3_MACMODESELECT_GMAC	(0 << 13)
 
+/* offsets from iface_base */
 #define MVPP22_SMI_MISC_CFG_REG			0x2a204
 #define      MVPP22_SMI_POLLING_EN		BIT(10)
 
@@ -4171,6 +4181,23 @@ static void mvpp2_port_mii_set(struct mvpp2_port *port)
 {
 	u32 val;
 
+	if (port->priv->hw_version == MVPP22) {
+		val = readl(port->base + MVPP22_XLG_CTRL3_REG);
+		val &= ~MVPP22_XLG_CTRL3_MACMODESELECT_MASK;
+		val |= MVPP22_XLG_CTRL3_MACMODESELECT_GMAC;
+		writel(val, port->base + MVPP22_XLG_CTRL3_REG);
+
+		val = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
+		if (port->phy_interface == PHY_INTERFACE_MODE_RGMII)
+			val |= MVPP22_CTRL4_EXT_PIN_GMII_SEL;
+		else
+			val &= ~MVPP22_CTRL4_EXT_PIN_GMII_SEL;
+		val &= ~MVPP22_CTRL4_DP_CLK_SEL;
+		val |= MVPP22_CTRL4_SYNC_BYPASS;
+		val |= MVPP22_CTRL4_QSGMII_BYPASS_ACTIVE;
+		writel(val, port->base + MVPP22_GMAC_CTRL_4_REG);
+	}
+
 	val = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
 
 	switch (port->phy_interface) {


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences

[Thomas Petazzoni]

Hello,

On Sat, 7 Jan 2017 09:38:34 +0000, Russell King - ARM Linux wrote:

> This has the side effect that nothing clears the port reset bit in the
> GMAC, which means there's no hope of the interface working - with the
> reset bit set, the port is well and truely held in "link down" state.

Things were working fine here on Armada 7K/8K even without calling
mvpp21_port_power_up(port). But I've looked into more details, and in
fact the whole function makes sense on PPv2.2, except the
mvpp2_port_fc_adv_enable(port) part of it. So I've replaced this with:

	mvpp2_port_mii_set(port);
	mvpp2_port_periodic_xon_disable(port);

	if (priv->hw_version == MVPP21)
		mvpp2_port_fc_adv_enable(port);

	mvpp2_port_reset(port);

This will be in my v3.

> In any case, the GMAC part is much the same as mvneta, and I think
> that code should be shared rather than writing new versions of it.

Possibly, but it's really a separate thing: this is completely
independent from introducing PPv2.2 support, it's something that
already exists. The patch series is already 16 patches long in its v2,
and is going to even a bit longer in its v3, so I would really like to
stick to just PPv2.2 support.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences

[Russell King - ARM Linux]

On Wed, Dec 28, 2016 at 05:46:27PM +0100, Thomas Petazzoni wrote:
> +#define MVPP22_SMI_MISC_CFG_REG			0x2a204
> +#define      MVPP22_SMI_POLLING_EN		BIT(10)
> +
...
> +	if (priv->hw_version == MVPP21) {
> +		val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
> +		val |= MVPP2_PHY_AN_STOP_SMI0_MASK;
> +		writel(val, priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
> +	} else {
> +		val = readl(priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
> +		val &= ~MVPP22_SMI_POLLING_EN;
> +		writel(val, priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
> +	}

The MVPP22_SMI_MISC_CFG_REG register is within the MDIO driver's
register set, although the mvmdio driver does not access this register.
Shouldn't this be taken care of by the mvmdio driver?

Also, a point that I've noticed while reviewing this is the mvmdio
driver also accesses these registers:

#define MVMDIO_ERR_INT_CAUSE               0x007C
#define MVMDIO_ERR_INT_MASK                0x0080

in addition to the un-named register at offset 0.  The driver writes
to these registers unconditionally when unbinding:

	writel(0, dev->regs + MVMDIO_ERR_INT_MASK);

However, the various bindings for the driver have:

arch/arm/boot/dts/armada-370-xp.dtsi:      compatible = "marvell,orion-mdio";
arch/arm/boot/dts/armada-370-xp.dtsi-      reg = <0x72004 0x4>;
arch/arm/boot/dts/armada-375.dtsi:         compatible = "marvell,orion-mdio";
arch/arm/boot/dts/armada-375.dtsi-         reg = <0xc0054 0x4>;
arch/arm/boot/dts/dove.dtsi:               compatible = "marvell,orion-mdio";
arch/arm/boot/dts/dove.dtsi-               #address-cells = <1>;
arch/arm/boot/dts/dove.dtsi-               #size-cells = <0>;
arch/arm/boot/dts/dove.dtsi-               reg = <0x72004 0x84>;
arch/arm/boot/dts/orion5x.dtsi:            compatible = "marvell,orion-mdio";
arch/arm/boot/dts/orion5x.dtsi-            #address-cells = <1>;
arch/arm/boot/dts/orion5x.dtsi-            #size-cells = <0>;
arch/arm/boot/dts/orion5x.dtsi-            reg = <0x72004 0x84>;
arch/arm/boot/dts/kirkwood.dtsi:           compatible = "marvell,orion-mdio";
arch/arm/boot/dts/kirkwood.dtsi-           #address-cells = <1>;
arch/arm/boot/dts/kirkwood.dtsi-           #size-cells = <0>;
arch/arm/boot/dts/kirkwood.dtsi-           reg = <0x72004 0x84>;
arch/arm/boot/dts/armada-38x.dtsi:         compatible = "marvell,orion-mdio";
arch/arm/boot/dts/armada-38x.dtsi-         reg = <0x72004 0x4>;

So, for many of these, we're accessing registers outside of the given
binding, which sounds incorrect.  I guess that write should be
conditional upon an interrupt being present.

The binding document says:

- reg: address and length of the SMI register

which is clearly wrong for those cases where the interrupt is used.

I also notice that the binding for CP110 uses a register size of 0x10
(even in your tree) - but I guess this should be 4.

I'm starting to wonder whether the orion-mdio driver really is a
separate chunk of hardware that warrants a separate description in
DT from the ethernet controller - it appears in all cases to be
embedded with an ethernet controller, sharing its register space
and at least some of the ethernet controllers clocks.  That says
to me that it isn't an independent functional unit of hardware.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences

[Marcin Wojtas]

Hi Russel,

2017-01-07 12:03 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Wed, Dec 28, 2016 at 05:46:27PM +0100, Thomas Petazzoni wrote:
>> +#define MVPP22_SMI_MISC_CFG_REG                      0x2a204
>> +#define      MVPP22_SMI_POLLING_EN           BIT(10)
>> +
> ...
>> +     if (priv->hw_version == MVPP21) {
>> +             val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
>> +             val |= MVPP2_PHY_AN_STOP_SMI0_MASK;
>> +             writel(val, priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
>> +     } else {
>> +             val = readl(priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
>> +             val &= ~MVPP22_SMI_POLLING_EN;
>> +             writel(val, priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
>> +     }
>
> The MVPP22_SMI_MISC_CFG_REG register is within the MDIO driver's
> register set, although the mvmdio driver does not access this register.
> Shouldn't this be taken care of by the mvmdio driver?
>
> Also, a point that I've noticed while reviewing this is the mvmdio
> driver also accesses these registers:
>
> #define MVMDIO_ERR_INT_CAUSE               0x007C
> #define MVMDIO_ERR_INT_MASK                0x0080
>
> in addition to the un-named register at offset 0.  The driver writes
> to these registers unconditionally when unbinding:
>
>         writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
>
> However, the various bindings for the driver have:
>
> arch/arm/boot/dts/armada-370-xp.dtsi:      compatible = "marvell,orion-mdio";
> arch/arm/boot/dts/armada-370-xp.dtsi-      reg = <0x72004 0x4>;
> arch/arm/boot/dts/armada-375.dtsi:         compatible = "marvell,orion-mdio";
> arch/arm/boot/dts/armada-375.dtsi-         reg = <0xc0054 0x4>;
> arch/arm/boot/dts/dove.dtsi:               compatible = "marvell,orion-mdio";
> arch/arm/boot/dts/dove.dtsi-               #address-cells = <1>;
> arch/arm/boot/dts/dove.dtsi-               #size-cells = <0>;
> arch/arm/boot/dts/dove.dtsi-               reg = <0x72004 0x84>;
> arch/arm/boot/dts/orion5x.dtsi:            compatible = "marvell,orion-mdio";
> arch/arm/boot/dts/orion5x.dtsi-            #address-cells = <1>;
> arch/arm/boot/dts/orion5x.dtsi-            #size-cells = <0>;
> arch/arm/boot/dts/orion5x.dtsi-            reg = <0x72004 0x84>;
> arch/arm/boot/dts/kirkwood.dtsi:           compatible = "marvell,orion-mdio";
> arch/arm/boot/dts/kirkwood.dtsi-           #address-cells = <1>;
> arch/arm/boot/dts/kirkwood.dtsi-           #size-cells = <0>;
> arch/arm/boot/dts/kirkwood.dtsi-           reg = <0x72004 0x84>;
> arch/arm/boot/dts/armada-38x.dtsi:         compatible = "marvell,orion-mdio";
> arch/arm/boot/dts/armada-38x.dtsi-         reg = <0x72004 0x4>;
>
> So, for many of these, we're accessing registers outside of the given
> binding, which sounds incorrect.  I guess that write should be
> conditional upon an interrupt being present.
>
> The binding document says:
>
> - reg: address and length of the SMI register
>
> which is clearly wrong for those cases where the interrupt is used.
>
> I also notice that the binding for CP110 uses a register size of 0x10
> (even in your tree) - but I guess this should be 4.
>
> I'm starting to wonder whether the orion-mdio driver really is a
> separate chunk of hardware that warrants a separate description in
> DT from the ethernet controller - it appears in all cases to be
> embedded with an ethernet controller, sharing its register space
> and at least some of the ethernet controllers clocks.  That says
> to me that it isn't an independent functional unit of hardware.
>

In fact there is common SMI bus, but each port has its own register
set to control it (it's true at least for Neta and PP2). There is also
an option to use HW polling - every 1s hardware checks PHY over SMI
and updates MAC registers of each port independently. I was able to
use those successfully in other implementations.

However we are supposed to use libphy in Linux and I'm afraid we have
to use single instance that controls single SMI bus - I think current
implementation is a compromise between HW and libphy demands.

Best regards,
Marcin

Re: [PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences

[Russell King - ARM Linux]

On Sat, Jan 07, 2017 at 01:12:35PM +0100, Marcin Wojtas wrote:
> In fact there is common SMI bus, but each port has its own register
> set to control it (it's true at least for Neta and PP2). There is also
> an option to use HW polling - every 1s hardware checks PHY over SMI
> and updates MAC registers of each port independently. I was able to
> use those successfully in other implementations.
> 
> However we are supposed to use libphy in Linux and I'm afraid we have
> to use single instance that controls single SMI bus - I think current
> implementation is a compromise between HW and libphy demands.

One of the "DT rules" is that DT only describes the hardware in an
implementation independent way.  It should not describe a particular
implementation's view of the hardware.

"libphy demands" sounds very much like an implementation dictating
the DT description, which sounds to me very wrong and against the
goals and purposes of DT.

Anyway, I don't think it's too bad - a possible solution here would
be to have the existing MDIO driver as a library, which can be
instantiated by the Neta and PP2 drivers.  This could be done in
DT (taking dove as an example) as:

                        eth: ethernet-ctrl@72000 {
                                compatible = "marvell,orion-eth";
				reg = <0x72000 0x4000>;
				ranges = <0 0x72000 0 0x4000>;
				...
				mdio: mdio@4 {
					compatible = "marvell,orion-mdio";
					reg = <4>;
					...
					... phys ...
				};
			};

I don't think that would require that big a change - and it could be
done in a way that compatibility with the existing DT descriptions is
maintained very cheaply.

Now, I'm not suggesting that mdio@4 should be created by a platform
device via marking ethernet-ctrl@72000 with "simple-bus", but it's
something that should be created by the ethernet driver if present.
The compatible string is there so we can identify that this is a
mdio node, and which type of mdio it is (the Armada 8k has this type
and a separate clause-45 xmdio implementation, and we need to
distinguish those.)

What that means is that we no longer have to worry about clocks and
overlapping register regions and the like, and can deal with the
ethernet driver wanting to access the SMI registers as well.

We would need the ethernet driver to be capable of instantiation even
with no ports enabled, so cases where the MDIO interface is used with
other ethernet controllers continues to be supportable (eg, the
Armada 8040 case where the slave CP110 ethernet controller is used
with PHYs connected to the master CP110 ethernet controller's MDIO
buses - which afaik aren't shared between the two CP110 dies.)

However, I'd like to see libphy become more flexible and support
hardware polled mode of operation, since libphy provides a nice
library of functions for accessing various phy features (like setting
the advertisment, EEE modes, flow control, etc.)  Even with hardware
polling, we should still describe the PHY in DT, because PHY is part
of the hardware description, and we need to know where it is in order
to program these phy features.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 net-next 12/16] net: mvpp2: add AXI bridge initialization for PPv2.2

[Thomas Petazzoni]

The PPv2.2 unit is connected to an AXI bus on Armada 7K/8K, so this
commit adds the necessary initialization of the AXI bridge.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 85 ++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index eb55576..d5b197d 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -157,6 +157,34 @@
 #define MVPP2_WIN_REMAP(w)			(0x4040 + ((w) << 2))
 #define MVPP2_BASE_ADDR_ENABLE			0x4060
 
+/* AXI Bridge Registers */
+#define MVPP22_AXI_BM_WR_ATTR_REG		0x4100
+#define MVPP22_AXI_BM_RD_ATTR_REG		0x4104
+#define MVPP22_AXI_AGGRQ_DESCR_RD_ATTR_REG	0x4110
+#define MVPP22_AXI_TXQ_DESCR_WR_ATTR_REG	0x4114
+#define MVPP22_AXI_TXQ_DESCR_RD_ATTR_REG	0x4118
+#define MVPP22_AXI_RXQ_DESCR_WR_ATTR_REG	0x411c
+#define MVPP22_AXI_RX_DATA_WR_ATTR_REG		0x4120
+#define MVPP22_AXI_TX_DATA_RD_ATTR_REG		0x4130
+#define MVPP22_AXI_RD_NORMAL_CODE_REG		0x4150
+#define MVPP22_AXI_RD_SNOOP_CODE_REG		0x4154
+#define MVPP22_AXI_WR_NORMAL_CODE_REG		0x4160
+#define MVPP22_AXI_WR_SNOOP_CODE_REG		0x4164
+
+/* Values for AXI Bridge registers */
+#define MVPP22_AXI_ATTR_CACHE_OFFS		0
+#define MVPP22_AXI_ATTR_DOMAIN_OFFS		12
+
+#define MVPP22_AXI_CODE_CACHE_OFFS		0
+#define MVPP22_AXI_CODE_DOMAIN_OFFS		4
+
+#define MVPP22_AXI_CODE_CACHE_NON_CACHE		0x3
+#define MVPP22_AXI_CODE_CACHE_WR_CACHE		0x7
+#define MVPP22_AXI_CODE_CACHE_RD_CACHE		0xb
+
+#define MVPP22_AXI_CODE_DOMAIN_OUTER_DOM	2
+#define MVPP22_AXI_CODE_DOMAIN_SYSTEM		3
+
 /* Interrupt Cause and Mask registers */
 #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)		(0x5200 + 4 * (rxq))
 #define     MVPP2_MAX_ISR_RX_THRESHOLD		0xfffff0
@@ -6640,6 +6668,60 @@ static void mvpp2_rx_fifo_init(struct mvpp2 *priv)
 	mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1);
 }
 
+static void mvpp2_axi_init(struct mvpp2 *priv)
+{
+	u32 val, rdval, wrval;
+
+	mvpp2_write(priv, MVPP22_BM_ADDR_HIGH_RLS_REG, 0x0);
+
+	/* AXI Bridge Configuration */
+
+	rdval = MVPP22_AXI_CODE_CACHE_RD_CACHE
+		<< MVPP22_AXI_ATTR_CACHE_OFFS;
+	rdval |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_ATTR_DOMAIN_OFFS;
+
+	wrval = MVPP22_AXI_CODE_CACHE_WR_CACHE
+		<< MVPP22_AXI_ATTR_CACHE_OFFS;
+	wrval |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_ATTR_DOMAIN_OFFS;
+
+	/* BM */
+	mvpp2_write(priv, MVPP22_AXI_BM_WR_ATTR_REG, wrval);
+	mvpp2_write(priv, MVPP22_AXI_BM_RD_ATTR_REG, rdval);
+
+	/* Descriptors */
+	mvpp2_write(priv, MVPP22_AXI_AGGRQ_DESCR_RD_ATTR_REG, rdval);
+	mvpp2_write(priv, MVPP22_AXI_TXQ_DESCR_WR_ATTR_REG, wrval);
+	mvpp2_write(priv, MVPP22_AXI_TXQ_DESCR_RD_ATTR_REG, rdval);
+	mvpp2_write(priv, MVPP22_AXI_RXQ_DESCR_WR_ATTR_REG, wrval);
+
+	/* Buffer Data */
+	mvpp2_write(priv, MVPP22_AXI_TX_DATA_RD_ATTR_REG, rdval);
+	mvpp2_write(priv, MVPP22_AXI_RX_DATA_WR_ATTR_REG, wrval);
+
+	val = MVPP22_AXI_CODE_CACHE_NON_CACHE
+		<< MVPP22_AXI_CODE_CACHE_OFFS;
+	val |= MVPP22_AXI_CODE_DOMAIN_SYSTEM
+		<< MVPP22_AXI_CODE_DOMAIN_OFFS;
+	mvpp2_write(priv, MVPP22_AXI_RD_NORMAL_CODE_REG, val);
+	mvpp2_write(priv, MVPP22_AXI_WR_NORMAL_CODE_REG, val);
+
+	val = MVPP22_AXI_CODE_CACHE_RD_CACHE
+		<< MVPP22_AXI_CODE_CACHE_OFFS;
+	val |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_CODE_DOMAIN_OFFS;
+
+	mvpp2_write(priv, MVPP22_AXI_RD_SNOOP_CODE_REG, val);
+
+	val = MVPP22_AXI_CODE_CACHE_WR_CACHE
+		<< MVPP22_AXI_CODE_CACHE_OFFS;
+	val |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_CODE_DOMAIN_OFFS;
+
+	mvpp2_write(priv, MVPP22_AXI_WR_SNOOP_CODE_REG, val);
+}
+
 /* Initialize network controller common part HW */
 static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 {
@@ -6659,6 +6741,9 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	if (dram_target_info)
 		mvpp2_conf_mbus_windows(dram_target_info, priv);
 
+	if (priv->hw_version == MVPP22)
+		mvpp2_axi_init(priv);
+
 	/* Disable HW PHY polling */
 	if (priv->hw_version == MVPP21) {
 		val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
-- 
2.7.4

[PATCHv2 net-next 13/16] net: mvpp2: rework RXQ interrupt group initialization for PPv2.2

[Thomas Petazzoni]

This commit adjusts how the MVPP2_ISR_RXQ_GROUP_REG register is
configured, since it changed between PPv2.1 and PPv2.2.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 45 ++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index d5b197d..baad991 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -188,7 +188,21 @@
 /* Interrupt Cause and Mask registers */
 #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)		(0x5200 + 4 * (rxq))
 #define     MVPP2_MAX_ISR_RX_THRESHOLD		0xfffff0
-#define MVPP2_ISR_RXQ_GROUP_REG(rxq)		(0x5400 + 4 * (rxq))
+#define MVPP21_ISR_RXQ_GROUP_REG(rxq)		(0x5400 + 4 * (rxq))
+
+#define MVPP22_ISR_RXQ_GROUP_INDEX_REG          0x5400
+#define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf
+#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_MASK   0x380
+#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET 7
+
+#define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf
+#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_MASK   0x380
+
+#define MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG     0x5404
+#define MVPP22_ISR_RXQ_SUB_GROUP_STARTQ_MASK    0x1f
+#define MVPP22_ISR_RXQ_SUB_GROUP_SIZE_MASK      0xf00
+#define MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET    8
+
 #define MVPP2_ISR_ENABLE_REG(port)		(0x5420 + 4 * (port))
 #define     MVPP2_ISR_ENABLE_INTERRUPT(mask)	((mask) & 0xffff)
 #define     MVPP2_ISR_DISABLE_INTERRUPT(mask)	(((mask) << 16) & 0xffff0000)
@@ -6385,7 +6399,18 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 	}
 
 	/* Configure Rx queue group interrupt for this port */
-	mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(port->id), rxq_number);
+	if (priv->hw_version == MVPP21)
+		mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(port->id),
+			    rxq_number);
+	else {
+		u32 val;
+
+		val = (port->id << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET);
+		mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val);
+
+		val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET);
+		mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val);
+	}
 
 	/* Create Rx descriptor rings */
 	for (queue = 0; queue < rxq_number; queue++) {
@@ -6775,8 +6800,20 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	mvpp2_rx_fifo_init(priv);
 
 	/* Reset Rx queue group interrupt configuration */
-	for (i = 0; i < MVPP2_MAX_PORTS; i++)
-		mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(i), rxq_number);
+	for (i = 0; i < MVPP2_MAX_PORTS; i++) {
+		if (priv->hw_version == MVPP21)
+			mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(i),
+				    rxq_number);
+		else {
+			u32 val;
+
+			val = (i << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET);
+			mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val);
+
+			val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET);
+			mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val);
+		}
+	}
 
 	if (priv->hw_version == MVPP21)
 		writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT,
-- 
2.7.4

[PATCHv2 net-next 15/16] net: mvpp2: add support for an additional clock needed for PPv2.2

[Thomas Petazzoni]

The PPv2.2 variant of the network controller needs an additional
clock, the "MG clock" in order for the IP block to operate
properly. This commit adds support for this additional clock to the
driver, reworking as needed the error handling path.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 20e9429..194de00 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -702,6 +702,7 @@ struct mvpp2 {
 	/* Common clocks */
 	struct clk *pp_clk;
 	struct clk *gop_clk;
+	struct clk *mg_clk;
 
 	/* List of pointers to port structures */
 	struct mvpp2_port **port_list;
@@ -6899,6 +6900,18 @@ static int mvpp2_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_pp_clk;
 
+	if (priv->hw_version == MVPP22) {
+		priv->mg_clk = devm_clk_get(&pdev->dev, "mg_clk");
+		if (IS_ERR(priv->mg_clk)) {
+			err = PTR_ERR(priv->mg_clk);
+			goto err_gop_clk;
+		}
+
+		err = clk_prepare_enable(priv->mg_clk);
+		if (err < 0)
+			goto err_gop_clk;
+	}
+
 	/* Get system's tclk rate */
 	priv->tclk = clk_get_rate(priv->pp_clk);
 
@@ -6906,14 +6919,14 @@ static int mvpp2_probe(struct platform_device *pdev)
 	err = mvpp2_init(pdev, priv);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to initialize controller\n");
-		goto err_gop_clk;
+		goto err_mg_clk;
 	}
 
 	port_count = of_get_available_child_count(dn);
 	if (port_count == 0) {
 		dev_err(&pdev->dev, "no ports enabled\n");
 		err = -ENODEV;
-		goto err_gop_clk;
+		goto err_mg_clk;
 	}
 
 	priv->port_list = devm_kcalloc(&pdev->dev, port_count,
@@ -6921,19 +6934,22 @@ static int mvpp2_probe(struct platform_device *pdev)
 				      GFP_KERNEL);
 	if (!priv->port_list) {
 		err = -ENOMEM;
-		goto err_gop_clk;
+		goto err_mg_clk;
 	}
 
 	/* Initialize ports */
 	for_each_available_child_of_node(dn, port_node) {
 		err = mvpp2_port_probe(pdev, port_node, priv);
 		if (err < 0)
-			goto err_gop_clk;
+			goto err_mg_clk;
 	}
 
 	platform_set_drvdata(pdev, priv);
 	return 0;
 
+err_mg_clk:
+	if (priv->hw_version == MVPP22)
+		clk_disable_unprepare(priv->mg_clk);
 err_gop_clk:
 	clk_disable_unprepare(priv->gop_clk);
 err_pp_clk:
@@ -6969,6 +6985,7 @@ static int mvpp2_remove(struct platform_device *pdev)
 				  aggr_txq->descs_phys);
 	}
 
+	clk_disable_unprepare(priv->mg_clk);
 	clk_disable_unprepare(priv->pp_clk);
 	clk_disable_unprepare(priv->gop_clk);
 
-- 
2.7.4

Re: [PATCHv2 net-next 15/16] net: mvpp2: add support for an additional clock needed for PPv2.2

[Russell King - ARM Linux]

On Wed, Dec 28, 2016 at 05:46:31PM +0100, Thomas Petazzoni wrote:
> The PPv2.2 variant of the network controller needs an additional
> clock, the "MG clock" in order for the IP block to operate
> properly. This commit adds support for this additional clock to the
> driver, reworking as needed the error handling path.

There's more clocks that are required.

Firstly, what I'm finding is that the MDIO block in the CP110 does not
work at all (locks the system up if accessed) if these clocks are not
enabled:

	cpm_syscon0 1 9, cpm_syscon0 1 6, cpm_syscon0 1 5

The XMDIO block requires the same clocks to be functional.  Since the
MDIO and XMDIO blocks are actually part of the ethernet controller,
it's not that surprising.

We can't rely on the ethernet controller having been probed, because
the 8k has two sets of MDIO buses - one set per CP110.  It's entirely
possible that people will put all their PHYs for both CP110 instances
on one set of MDIO buses (eg, the master).  Indeed, this is exactly
what SolidRun have done, and hence how I found the problem.

So, it looks to me like DT doesn't actually describe the hardware here -
especially when looking at the reg properties, they overlap with each
other.

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/net/ethernet/marvell/mvpp2.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
> index 20e9429..194de00 100644
> --- a/drivers/net/ethernet/marvell/mvpp2.c
> +++ b/drivers/net/ethernet/marvell/mvpp2.c
> @@ -702,6 +702,7 @@ struct mvpp2 {
>  	/* Common clocks */
>  	struct clk *pp_clk;
>  	struct clk *gop_clk;
> +	struct clk *mg_clk;
>  
>  	/* List of pointers to port structures */
>  	struct mvpp2_port **port_list;
> @@ -6899,6 +6900,18 @@ static int mvpp2_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto err_pp_clk;
>  
> +	if (priv->hw_version == MVPP22) {
> +		priv->mg_clk = devm_clk_get(&pdev->dev, "mg_clk");
> +		if (IS_ERR(priv->mg_clk)) {
> +			err = PTR_ERR(priv->mg_clk);
> +			goto err_gop_clk;
> +		}
> +
> +		err = clk_prepare_enable(priv->mg_clk);
> +		if (err < 0)
> +			goto err_gop_clk;
> +	}
> +
>  	/* Get system's tclk rate */
>  	priv->tclk = clk_get_rate(priv->pp_clk);
>  
> @@ -6906,14 +6919,14 @@ static int mvpp2_probe(struct platform_device *pdev)
>  	err = mvpp2_init(pdev, priv);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "failed to initialize controller\n");
> -		goto err_gop_clk;
> +		goto err_mg_clk;
>  	}
>  
>  	port_count = of_get_available_child_count(dn);
>  	if (port_count == 0) {
>  		dev_err(&pdev->dev, "no ports enabled\n");
>  		err = -ENODEV;
> -		goto err_gop_clk;
> +		goto err_mg_clk;
>  	}
>  
>  	priv->port_list = devm_kcalloc(&pdev->dev, port_count,
> @@ -6921,19 +6934,22 @@ static int mvpp2_probe(struct platform_device *pdev)
>  				      GFP_KERNEL);
>  	if (!priv->port_list) {
>  		err = -ENOMEM;
> -		goto err_gop_clk;
> +		goto err_mg_clk;
>  	}
>  
>  	/* Initialize ports */
>  	for_each_available_child_of_node(dn, port_node) {
>  		err = mvpp2_port_probe(pdev, port_node, priv);
>  		if (err < 0)
> -			goto err_gop_clk;
> +			goto err_mg_clk;
>  	}
>  
>  	platform_set_drvdata(pdev, priv);
>  	return 0;
>  
> +err_mg_clk:
> +	if (priv->hw_version == MVPP22)
> +		clk_disable_unprepare(priv->mg_clk);
>  err_gop_clk:
>  	clk_disable_unprepare(priv->gop_clk);
>  err_pp_clk:
> @@ -6969,6 +6985,7 @@ static int mvpp2_remove(struct platform_device *pdev)
>  				  aggr_txq->descs_phys);
>  	}
>  
> +	clk_disable_unprepare(priv->mg_clk);
>  	clk_disable_unprepare(priv->pp_clk);
>  	clk_disable_unprepare(priv->gop_clk);
>  
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html