Message ID | 20250124122353.1457174-1-basharath@couthit.com |
---|---|
Headers | show |
Series | PRU-ICSSM Ethernet Driver | expand |
On Fri, Jan 24, 2025 at 07:10:50PM +0530, Basharath Hussain Khaja wrote: > From: Roger Quadros <rogerq@ti.com> > > Changes corresponding to link configuration such as speed and duplexity. > IRQ and handler initializations are performed for packet reception.Firmware > receives the packet from the wire and stores it into OCMC queue. Next, it > notifies the CPU via interrupt. Upon receiving the interrupt CPU will > service the IRQ and packet will be processed by pushing the newly allocated > SKB to upper layers. > > When the user application want to transmit a packet, it will invoke > sys_send() which will inturn invoke the PRUETH driver, then it will write > the packet into OCMC queues. PRU firmware will pick up the packet and > transmit it on to the wire. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Signed-off-by: Andrew F. Davis <afd@ti.com> > Signed-off-by: Parvathi Pudi <parvathi@couthit.com> > Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> > --- > drivers/net/ethernet/ti/icssm/icssm_prueth.c | 599 ++++++++++++++++++- > drivers/net/ethernet/ti/icssm/icssm_prueth.h | 46 ++ > 2 files changed, 640 insertions(+), 5 deletions(-) Looks like this patch was duplicated and posted twice ?
> On Fri, Jan 24, 2025 at 05:53:44PM +0530, Basharath Hussain Khaja wrote: >> From: Parvathi Pudi <parvathi@couthit.com> >> >> Documentation update for the newly added "pruss2_eth" device tree >> node and its dependencies along with compatibility for PRU-ICSS >> Industrial Ethernet Peripheral (IEP), PRU-ICSS Enhanced Capture >> (eCAP) peripheral and using YAML binding document for AM57xx SoCs. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> Signed-off-by: Parvathi Pudi <parvathi@couthit.com> >> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> > > I find this hard to believe. If all these people handled the patch, the > signoff from Parvathi would be first, no? Should some of these people be > co-developers? > Changes are about multiple modules. We have added our sign-off followed by original module authors. >> --- >> .../devicetree/bindings/net/ti,icss-iep.yaml | 5 + >> .../bindings/net/ti,icssm-prueth.yaml | 147 ++++++++++++++++++ >> .../bindings/net/ti,pruss-ecap.yaml | 32 ++++ >> .../devicetree/bindings/soc/ti/ti,pruss.yaml | 9 ++ >> 4 files changed, 193 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> create mode 100644 Documentation/devicetree/bindings/net/ti,pruss-ecap.yaml >> >> diff --git a/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> b/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> index e36e3a622904..aad7d37fb47e 100644 >> --- a/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> +++ b/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> @@ -8,6 +8,8 @@ title: Texas Instruments ICSS Industrial Ethernet Peripheral >> (IEP) module >> >> maintainers: >> - Md Danish Anwar <danishanwar@ti.com> >> + - Parvathi Pudi <parvathi@couthit.com> >> + - Basharath Hussain Khaja <basharath@couthit.com> >> >> properties: >> compatible: >> @@ -20,6 +22,9 @@ properties: >> >> - const: ti,am654-icss-iep >> >> + - items: >> + - enum: >> + - ti,am5728-icss-iep > > "items: - enum: <one item>" is the same as const. > Sure, we will modify as below. - const: ti,am5728-icss-iep >> >> reg: >> maxItems: 1 >> diff --git a/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> b/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> new file mode 100644 >> index 000000000000..51e99beb5f5f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> @@ -0,0 +1,147 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/net/ti,icssm-prueth.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Texas Instruments ICSSM PRUSS Ethernet >> + >> +maintainers: >> + - Roger Quadros <rogerq@ti.com> >> + - Andrew F. Davis <afd@ti.com> >> + - Parvathi Pudi <parvathi@couthit.com> >> + - Basharath Hussain Khaja <basharath@couthit.com> >> + >> +description: >> + Ethernet based on the Programmable Real-Time Unit and Industrial >> + Communication Subsystem. >> + >> +properties: >> + compatible: >> + enum: >> + - ti,am57-prueth # for AM57x SoC family >> + >> + sram: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + phandle to OCMC SRAM node >> + >> + ti,mii-rt: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + phandle to MII_RT module's syscon regmap >> + >> + ti,iep: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + phandle to IEP (Industrial Ethernet Peripheral) for ICSS >> + >> + ecap: > > Why's this one not got a ti prefix? > We will add "ti" prefix to ecap as "ti,ecap" in the next version. >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + phandle to Enhanced Capture (eCAP) event for ICSS > > Why do you need phandles for these things, can they not be looked up by > compatible? (e.g. multiple devices on one SoC). > ecap is another peripheral similar to IEP in ICSSM/ICSSG. We have created a separate driver for possible reuse with ICSSG in future. >> + >> + interrupts: >> + items: >> + - description: High priority Rx Interrupt specifier. > > + - description: Low priority Rx Interrupt specifier. Thanks & Best Regards, Basharath
> On Fri, Jan 24, 2025 at 07:10:50PM +0530, Basharath Hussain Khaja wrote: >> From: Roger Quadros <rogerq@ti.com> >> >> Changes corresponding to link configuration such as speed and duplexity. >> IRQ and handler initializations are performed for packet reception.Firmware >> receives the packet from the wire and stores it into OCMC queue. Next, it >> notifies the CPU via interrupt. Upon receiving the interrupt CPU will >> service the IRQ and packet will be processed by pushing the newly allocated >> SKB to upper layers. >> >> When the user application want to transmit a packet, it will invoke >> sys_send() which will inturn invoke the PRUETH driver, then it will write >> the packet into OCMC queues. PRU firmware will pick up the packet and >> transmit it on to the wire. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> Signed-off-by: Parvathi Pudi <parvathi@couthit.com> >> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> >> --- >> drivers/net/ethernet/ti/icssm/icssm_prueth.c | 599 ++++++++++++++++++- >> drivers/net/ethernet/ti/icssm/icssm_prueth.h | 46 ++ >> 2 files changed, 640 insertions(+), 5 deletions(-) > > Looks like this patch was duplicated and posted twice ? When I posted the patch first time I received a bounce back error. So I was not sure if the patch got posted successfully or not so I have to re-sent. Sorry for the confusion. Thanks & Best Regards, Basharath.
On Wed, Jan 29, 2025 at 10:46:52AM +0530, Basharath Hussain Khaja wrote: > > On Fri, Jan 24, 2025 at 05:53:44PM +0530, Basharath Hussain Khaja wrote: > >> From: Parvathi Pudi <parvathi@couthit.com> > >> > >> Documentation update for the newly added "pruss2_eth" device tree > >> node and its dependencies along with compatibility for PRU-ICSS > >> Industrial Ethernet Peripheral (IEP), PRU-ICSS Enhanced Capture > >> (eCAP) peripheral and using YAML binding document for AM57xx SoCs. > >> > >> Signed-off-by: Roger Quadros <rogerq@ti.com> > >> Signed-off-by: Andrew F. Davis <afd@ti.com> > >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> > >> Signed-off-by: Parvathi Pudi <parvathi@couthit.com> > >> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> > > > > I find this hard to believe. If all these people handled the patch, the > > signoff from Parvathi would be first, no? Should some of these people be > > co-developers? > > > > Changes are about multiple modules. We have added our sign-off followed by original module authors. I think what you're trying to say is that these people are co-developers? Anyone that contributed to the content of this patch needs to get a co-developed-by. If they're not co-developers, and you just want to put them in the maintainers section, they don't get sign-offs. > >> --- > >> .../devicetree/bindings/net/ti,icss-iep.yaml | 5 + > >> .../bindings/net/ti,icssm-prueth.yaml | 147 ++++++++++++++++++ > >> .../bindings/net/ti,pruss-ecap.yaml | 32 ++++ > >> .../devicetree/bindings/soc/ti/ti,pruss.yaml | 9 ++ > >> 4 files changed, 193 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml > >> create mode 100644 Documentation/devicetree/bindings/net/ti,pruss-ecap.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/net/ti,icss-iep.yaml > >> b/Documentation/devicetree/bindings/net/ti,icss-iep.yaml > >> index e36e3a622904..aad7d37fb47e 100644 > >> --- a/Documentation/devicetree/bindings/net/ti,icss-iep.yaml > >> +++ b/Documentation/devicetree/bindings/net/ti,icss-iep.yaml > >> @@ -8,6 +8,8 @@ title: Texas Instruments ICSS Industrial Ethernet Peripheral > >> (IEP) module > >> > >> maintainers: > >> - Md Danish Anwar <danishanwar@ti.com> > >> + - Parvathi Pudi <parvathi@couthit.com> > >> + - Basharath Hussain Khaja <basharath@couthit.com> > >> > >> properties: > >> compatible: > >> @@ -20,6 +22,9 @@ properties: > >> > >> - const: ti,am654-icss-iep > >> > >> + - items: > >> + - enum: > >> + - ti,am5728-icss-iep > > > > "items: - enum: <one item>" is the same as const. > > > > Sure, we will modify as below. > > - const: ti,am5728-icss-iep > > >> > >> reg: > >> maxItems: 1 > >> diff --git a/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml > >> b/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml > >> new file mode 100644 > >> index 000000000000..51e99beb5f5f > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml > >> @@ -0,0 +1,147 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id: http://devicetree.org/schemas/net/ti,icssm-prueth.yaml# > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: Texas Instruments ICSSM PRUSS Ethernet > >> + > >> +maintainers: > >> + - Roger Quadros <rogerq@ti.com> > >> + - Andrew F. Davis <afd@ti.com> > >> + - Parvathi Pudi <parvathi@couthit.com> > >> + - Basharath Hussain Khaja <basharath@couthit.com> > >> + > >> +description: > >> + Ethernet based on the Programmable Real-Time Unit and Industrial > >> + Communication Subsystem. > >> + > >> +properties: > >> + compatible: > >> + enum: > >> + - ti,am57-prueth # for AM57x SoC family > >> + > >> + sram: > >> + $ref: /schemas/types.yaml#/definitions/phandle > >> + description: > >> + phandle to OCMC SRAM node > >> + > >> + ti,mii-rt: > >> + $ref: /schemas/types.yaml#/definitions/phandle > >> + description: > >> + phandle to MII_RT module's syscon regmap > >> + > >> + ti,iep: > >> + $ref: /schemas/types.yaml#/definitions/phandle > >> + description: > >> + phandle to IEP (Industrial Ethernet Peripheral) for ICSS > >> + > >> + ecap: > > > > Why's this one not got a ti prefix? > > > > We will add "ti" prefix to ecap as "ti,ecap" in the next version. > > >> + $ref: /schemas/types.yaml#/definitions/phandle > >> + description: > >> + phandle to Enhanced Capture (eCAP) event for ICSS > > > > Why do you need phandles for these things, can they not be looked up by > > compatible? (e.g. multiple devices on one SoC). > > > > ecap is another peripheral similar to IEP in ICSSM/ICSSG. We have created a separate driver for possible reuse with ICSSG in future. That's not an answer to my question. > > >> + > >> + interrupts: > >> + items: > >> + - description: High priority Rx Interrupt specifier. > > > + - description: Low priority Rx Interrupt specifier. > > > Thanks & Best Regards, > Basharath
On Fri, Jan 24, 2025 at 05:53:46PM +0530, Basharath Hussain Khaja wrote: > From: Roger Quadros <rogerq@ti.com> > > Updates for MII_RT hardware peripheral configuration such as RX and TX > configuration for PRU0 and PRU1, frame sizes, and MUX config. > > Updates for PRU-ICSS firmware register configuration and DRAM, SRAM and > OCMC memory initialization, which will be used in the runtime for packet > reception and transmission. > > DUAL-EMAC memory allocation for software queues and its supporting > components such as the buffer descriptors and queue discriptors. These nit: descriptors > software queues are placed in OCMC memory and are shared with CPU by > PRU-ICSS for packet receive and transmit. > > All declarations and macros are being used from common header file > for various protocols. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Signed-off-by: Andrew F. Davis <afd@ti.com> > Signed-off-by: Parvathi Pudi <parvathi@couthit.com> > Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> ... > diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c b/drivers/net/ethernet/ti/icssm/icssm_prueth.c ... > +static void icssm_prueth_mii_init(struct prueth *prueth) > +{ > + struct regmap *mii_rt; > + u32 rxcfg_reg, rxcfg; > + u32 txcfg_reg, txcfg; > + > + mii_rt = prueth->mii_rt; > + > + rxcfg = PRUSS_MII_RT_RXCFG_RX_ENABLE | > + PRUSS_MII_RT_RXCFG_RX_DATA_RDY_MODE_DIS | > + PRUSS_MII_RT_RXCFG_RX_L2_EN | > + PRUSS_MII_RT_RXCFG_RX_CUT_PREAMBLE | > + PRUSS_MII_RT_RXCFG_RX_L2_EOF_SCLR_DIS; > + > + /* Configuration of Port 0 Rx */ > + rxcfg_reg = PRUSS_MII_RT_RXCFG0; > + > + regmap_write(mii_rt, rxcfg_reg, rxcfg); > + > + /* Configuration of Port 1 Rx */ > + rxcfg_reg = PRUSS_MII_RT_RXCFG1; > + > + rxcfg |= PRUSS_MII_RT_RXCFG_RX_MUX_SEL; > + > + regmap_write(mii_rt, rxcfg_reg, rxcfg); > + > + txcfg = PRUSS_MII_RT_TXCFG_TX_ENABLE | > + PRUSS_MII_RT_TXCFG_TX_AUTO_PREAMBLE | > + PRUSS_MII_RT_TXCFG_TX_32_MODE_EN | > + (TX_START_DELAY << PRUSS_MII_RT_TXCFG_TX_START_DELAY_SHIFT) | > + (TX_CLK_DELAY_100M << PRUSS_MII_RT_TXCFG_TX_CLK_DELAY_SHIFT); > + > + /* Configuration of Port 0 Tx */ > + txcfg_reg = PRUSS_MII_RT_TXCFG0; > + > + regmap_write(mii_rt, txcfg_reg, txcfg); > + > + txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL; nit: a space seems more appropriate than a tab before '|=' > + > + /* Configuration of Port 1 Tx */ > + txcfg_reg = PRUSS_MII_RT_TXCFG1; > + > + regmap_write(mii_rt, txcfg_reg, txcfg); > + > + txcfg_reg = PRUSS_MII_RT_RX_FRMS0; > + > + /* Min frame length should be set to 64 to allow receive of standard > + * Ethernet frames such as PTP, LLDP that will not have the tag/rct. > + * Actual size written to register is size - 1 per TRM. This also > + * includes CRC/FCS. > + */ > + txcfg = (((PRUSS_MII_RT_RX_FRMS_MIN_FRM - 1) << > + PRUSS_MII_RT_RX_FRMS_MIN_FRM_SHIFT) & > + PRUSS_MII_RT_RX_FRMS_MIN_FRM_MASK); > + > + /* For EMAC, set Max frame size to 1528 i.e size with VLAN. > + * Actual size written to register is size - 1 as per TRM. > + * Since driver support run time change of protocol, driver > + * must overwrite the values based on Ethernet type. > + */ > + txcfg |= (((PRUSS_MII_RT_RX_FRMS_MAX_SUPPORT_EMAC - 1) << > + PRUSS_MII_RT_RX_FRMS_MAX_FRM_SHIFT) & > + PRUSS_MII_RT_RX_FRMS_MAX_FRM_MASK); > + > + regmap_write(mii_rt, txcfg_reg, txcfg); > + > + txcfg_reg = PRUSS_MII_RT_RX_FRMS1; > + > + regmap_write(mii_rt, txcfg_reg, txcfg); > +} ... > @@ -377,6 +705,70 @@ static int icssm_prueth_probe(struct platform_device *pdev) > } > } > > + pruss = pruss_get(prueth->pru0 ? prueth->pru0 : prueth->pru1); > + if (IS_ERR(pruss)) { > + ret = PTR_ERR(pruss); > + dev_err(dev, "unable to get pruss handle\n"); > + goto put_pru; > + } > + prueth->pruss = pruss; > + > + ret = pruss_cfg_ocp_master_ports(prueth->pruss, 1); > + if (ret) { > + dev_err(dev, "couldn't enabled ocp master port: %d\n", ret); > + goto put_pruss; > + } FTR, I applied this patch set on top of the patch at the link below so that pruss_cfg_ocp_master_ports() is present. - [PATCH v2 1/1] soc: ti: PRUSS OCP configuration https://lore.kernel.org/all/20250108125937.10604-2-basharath@couthit.com/ ... > diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.h b/drivers/net/ethernet/ti/icssm/icssm_prueth.h ... > +/** > + * struct prueth_queue - Information about a queue in memory struct prueth_queue_info > + * @buffer_offset: buffer offset in OCMC RAM > + * @queue_desc_offset: queue descriptor offset in Shared RAM > + * @buffer_desc_offset: buffer descriptors offset in Shared RAM > + * @buffer_desc_end: end address of buffer descriptors in Shared RAM > + */ > +struct prueth_queue_info { > + u16 buffer_offset; > + u16 queue_desc_offset; > + u16 buffer_desc_offset; > + u16 buffer_desc_end; > +} __packed; ...
> On Fri, Jan 24, 2025 at 05:53:46PM +0530, Basharath Hussain Khaja wrote: >> From: Roger Quadros <rogerq@ti.com> >> >> Updates for MII_RT hardware peripheral configuration such as RX and TX >> configuration for PRU0 and PRU1, frame sizes, and MUX config. >> >> Updates for PRU-ICSS firmware register configuration and DRAM, SRAM and >> OCMC memory initialization, which will be used in the runtime for packet >> reception and transmission. >> >> DUAL-EMAC memory allocation for software queues and its supporting >> components such as the buffer descriptors and queue discriptors. These > > nit: descriptors > Sure. We will change in the next version. >> software queues are placed in OCMC memory and are shared with CPU by >> PRU-ICSS for packet receive and transmit. >> >> All declarations and macros are being used from common header file >> for various protocols. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> Signed-off-by: Parvathi Pudi <parvathi@couthit.com> >> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> > > ... > >> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c >> b/drivers/net/ethernet/ti/icssm/icssm_prueth.c > > ... > >> +static void icssm_prueth_mii_init(struct prueth *prueth) >> +{ >> + struct regmap *mii_rt; >> + u32 rxcfg_reg, rxcfg; >> + u32 txcfg_reg, txcfg; >> + >> + mii_rt = prueth->mii_rt; >> + >> + rxcfg = PRUSS_MII_RT_RXCFG_RX_ENABLE | >> + PRUSS_MII_RT_RXCFG_RX_DATA_RDY_MODE_DIS | >> + PRUSS_MII_RT_RXCFG_RX_L2_EN | >> + PRUSS_MII_RT_RXCFG_RX_CUT_PREAMBLE | >> + PRUSS_MII_RT_RXCFG_RX_L2_EOF_SCLR_DIS; >> + >> + /* Configuration of Port 0 Rx */ >> + rxcfg_reg = PRUSS_MII_RT_RXCFG0; >> + >> + regmap_write(mii_rt, rxcfg_reg, rxcfg); >> + >> + /* Configuration of Port 1 Rx */ >> + rxcfg_reg = PRUSS_MII_RT_RXCFG1; >> + >> + rxcfg |= PRUSS_MII_RT_RXCFG_RX_MUX_SEL; >> + >> + regmap_write(mii_rt, rxcfg_reg, rxcfg); >> + >> + txcfg = PRUSS_MII_RT_TXCFG_TX_ENABLE | >> + PRUSS_MII_RT_TXCFG_TX_AUTO_PREAMBLE | >> + PRUSS_MII_RT_TXCFG_TX_32_MODE_EN | >> + (TX_START_DELAY << PRUSS_MII_RT_TXCFG_TX_START_DELAY_SHIFT) | >> + (TX_CLK_DELAY_100M << PRUSS_MII_RT_TXCFG_TX_CLK_DELAY_SHIFT); >> + >> + /* Configuration of Port 0 Tx */ >> + txcfg_reg = PRUSS_MII_RT_TXCFG0; >> + >> + regmap_write(mii_rt, txcfg_reg, txcfg); >> + >> + txcfg |= PRUSS_MII_RT_TXCFG_TX_MUX_SEL; > > nit: a space seems more appropriate than a tab before '|=' > Sure. We will remove extra spaces in the next version. >> + >> + /* Configuration of Port 1 Tx */ >> + txcfg_reg = PRUSS_MII_RT_TXCFG1; >> + >> + regmap_write(mii_rt, txcfg_reg, txcfg); >> + >> + txcfg_reg = PRUSS_MII_RT_RX_FRMS0; >> + >> + /* Min frame length should be set to 64 to allow receive of standard >> + * Ethernet frames such as PTP, LLDP that will not have the tag/rct. >> + * Actual size written to register is size - 1 per TRM. This also >> + * includes CRC/FCS. >> + */ >> + txcfg = (((PRUSS_MII_RT_RX_FRMS_MIN_FRM - 1) << >> + PRUSS_MII_RT_RX_FRMS_MIN_FRM_SHIFT) & >> + PRUSS_MII_RT_RX_FRMS_MIN_FRM_MASK); >> + >> + /* For EMAC, set Max frame size to 1528 i.e size with VLAN. >> + * Actual size written to register is size - 1 as per TRM. >> + * Since driver support run time change of protocol, driver >> + * must overwrite the values based on Ethernet type. >> + */ >> + txcfg |= (((PRUSS_MII_RT_RX_FRMS_MAX_SUPPORT_EMAC - 1) << >> + PRUSS_MII_RT_RX_FRMS_MAX_FRM_SHIFT) & >> + PRUSS_MII_RT_RX_FRMS_MAX_FRM_MASK); >> + >> + regmap_write(mii_rt, txcfg_reg, txcfg); >> + >> + txcfg_reg = PRUSS_MII_RT_RX_FRMS1; >> + >> + regmap_write(mii_rt, txcfg_reg, txcfg); >> +} > > ... > >> @@ -377,6 +705,70 @@ static int icssm_prueth_probe(struct platform_device *pdev) >> } >> } >> >> + pruss = pruss_get(prueth->pru0 ? prueth->pru0 : prueth->pru1); >> + if (IS_ERR(pruss)) { >> + ret = PTR_ERR(pruss); >> + dev_err(dev, "unable to get pruss handle\n"); >> + goto put_pru; >> + } >> + prueth->pruss = pruss; >> + >> + ret = pruss_cfg_ocp_master_ports(prueth->pruss, 1); >> + if (ret) { >> + dev_err(dev, "couldn't enabled ocp master port: %d\n", ret); >> + goto put_pruss; >> + } > > FTR, I applied this patch set on top of the patch at the link below > so that pruss_cfg_ocp_master_ports() is present. > > - [PATCH v2 1/1] soc: ti: PRUSS OCP configuration > https://lore.kernel.org/all/20250108125937.10604-2-basharath@couthit.com/ > > ... > >> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.h >> b/drivers/net/ethernet/ti/icssm/icssm_prueth.h > > ... > >> +/** >> + * struct prueth_queue - Information about a queue in memory > > struct prueth_queue_info > We will address this in the next version. >> + * @buffer_offset: buffer offset in OCMC RAM >> + * @queue_desc_offset: queue descriptor offset in Shared RAM >> + * @buffer_desc_offset: buffer descriptors offset in Shared RAM >> + * @buffer_desc_end: end address of buffer descriptors in Shared RAM >> + */ >> +struct prueth_queue_info { >> + u16 buffer_offset; >> + u16 queue_desc_offset; >> + u16 buffer_desc_offset; >> + u16 buffer_desc_end; >> +} __packed; > > ... Thanks & Best Regards, Basharath
> On Wed, Jan 29, 2025 at 10:46:52AM +0530, Basharath Hussain Khaja wrote: >> > On Fri, Jan 24, 2025 at 05:53:44PM +0530, Basharath Hussain Khaja wrote: >> >> From: Parvathi Pudi <parvathi@couthit.com> >> >> >> >> Documentation update for the newly added "pruss2_eth" device tree >> >> node and its dependencies along with compatibility for PRU-ICSS >> >> Industrial Ethernet Peripheral (IEP), PRU-ICSS Enhanced Capture >> >> (eCAP) peripheral and using YAML binding document for AM57xx SoCs. >> >> >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> >> Signed-off-by: Parvathi Pudi <parvathi@couthit.com> >> >> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> >> > >> > I find this hard to believe. If all these people handled the patch, the >> > signoff from Parvathi would be first, no? Should some of these people be >> > co-developers? >> > >> >> Changes are about multiple modules. We have added our sign-off followed by >> original module authors. > > I think what you're trying to say is that these people are > co-developers? Anyone that contributed to the content of this patch > needs to get a co-developed-by. If they're not co-developers, and you > just want to put them in the maintainers section, they don't get > sign-offs. > Yes you may be right. We thought it would be good to include module owners in signed-off-by, though it is a documentation file which was not available earlier we have newly added. Due to that the ownership is with us. As you suggested we will clean this in the next version as below. Signed-off-by: Parvathi Pudi <parvathi@couthit.com> Signed-off-by: Basharath Hussain Khaja <basharath@couthit.com> >> >> --- >> >> .../devicetree/bindings/net/ti,icss-iep.yaml | 5 + >> >> .../bindings/net/ti,icssm-prueth.yaml | 147 ++++++++++++++++++ >> >> .../bindings/net/ti,pruss-ecap.yaml | 32 ++++ >> >> .../devicetree/bindings/soc/ti/ti,pruss.yaml | 9 ++ >> >> 4 files changed, 193 insertions(+) >> >> create mode 100644 Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> >> create mode 100644 Documentation/devicetree/bindings/net/ti,pruss-ecap.yaml >> >> >> >> diff --git a/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> >> b/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> >> index e36e3a622904..aad7d37fb47e 100644 >> >> --- a/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> >> +++ b/Documentation/devicetree/bindings/net/ti,icss-iep.yaml >> >> @@ -8,6 +8,8 @@ title: Texas Instruments ICSS Industrial Ethernet Peripheral >> >> (IEP) module >> >> >> >> maintainers: >> >> - Md Danish Anwar <danishanwar@ti.com> >> >> + - Parvathi Pudi <parvathi@couthit.com> >> >> + - Basharath Hussain Khaja <basharath@couthit.com> >> >> >> >> properties: >> >> compatible: >> >> @@ -20,6 +22,9 @@ properties: >> >> >> >> - const: ti,am654-icss-iep >> >> >> >> + - items: >> >> + - enum: >> >> + - ti,am5728-icss-iep >> > >> > "items: - enum: <one item>" is the same as const. >> > >> >> Sure, we will modify as below. >> >> - const: ti,am5728-icss-iep >> >> >> >> >> reg: >> >> maxItems: 1 >> >> diff --git a/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> >> b/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> >> new file mode 100644 >> >> index 000000000000..51e99beb5f5f >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/net/ti,icssm-prueth.yaml >> >> @@ -0,0 +1,147 @@ >> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> >> +%YAML 1.2 >> >> +--- >> >> +$id: http://devicetree.org/schemas/net/ti,icssm-prueth.yaml# >> >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> >> + >> >> +title: Texas Instruments ICSSM PRUSS Ethernet >> >> + >> >> +maintainers: >> >> + - Roger Quadros <rogerq@ti.com> >> >> + - Andrew F. Davis <afd@ti.com> >> >> + - Parvathi Pudi <parvathi@couthit.com> >> >> + - Basharath Hussain Khaja <basharath@couthit.com> >> >> + >> >> +description: >> >> + Ethernet based on the Programmable Real-Time Unit and Industrial >> >> + Communication Subsystem. >> >> + >> >> +properties: >> >> + compatible: >> >> + enum: >> >> + - ti,am57-prueth # for AM57x SoC family >> >> + >> >> + sram: >> >> + $ref: /schemas/types.yaml#/definitions/phandle >> >> + description: >> >> + phandle to OCMC SRAM node >> >> + >> >> + ti,mii-rt: >> >> + $ref: /schemas/types.yaml#/definitions/phandle >> >> + description: >> >> + phandle to MII_RT module's syscon regmap >> >> + >> >> + ti,iep: >> >> + $ref: /schemas/types.yaml#/definitions/phandle >> >> + description: >> >> + phandle to IEP (Industrial Ethernet Peripheral) for ICSS >> >> + >> >> + ecap: >> > >> > Why's this one not got a ti prefix? >> > >> >> We will add "ti" prefix to ecap as "ti,ecap" in the next version. >> >> >> + $ref: /schemas/types.yaml#/definitions/phandle >> >> + description: >> >> + phandle to Enhanced Capture (eCAP) event for ICSS >> > >> > Why do you need phandles for these things, can they not be looked up by >> > compatible? (e.g. multiple devices on one SoC). >> > >> >> ecap is another peripheral similar to IEP in ICSSM/ICSSG. We have created a >> separate driver for possible reuse with ICSSG in future. > > That's not an answer to my question. > We can use compatible if we have only one instance of a peripheral in the SOC. On the AM57x SOC we have two identical ICSS instances(ICSS1 and ICSS2). So we use phandles to differentiate between the two instances. Currently this patch series adds support for ICSS2 instance on the AM57x SOC. Support for ICSS1 instance will be added in subsequent patches. >> >> >> + >> >> + interrupts: >> >> + items: >> >> + - description: High priority Rx Interrupt specifier. >> > > + - description: Low priority Rx Interrupt specifier. >> >> Thanks & Best Regards, Basharath
On Mon, Feb 03, 2025 at 05:59:55PM +0530, Basharath Hussain Khaja wrote: > >> >> + $ref: /schemas/types.yaml#/definitions/phandle > >> >> + description: > >> >> + phandle to Enhanced Capture (eCAP) event for ICSS > >> > > >> > Why do you need phandles for these things, can they not be looked up by > >> > compatible? (e.g. multiple devices on one SoC). > >> > > >> > >> ecap is another peripheral similar to IEP in ICSSM/ICSSG. We have created a > >> separate driver for possible reuse with ICSSG in future. > > > > That's not an answer to my question. > > > > We can use compatible if we have only one instance of a peripheral in the SOC. > On the AM57x SOC we have two identical ICSS instances(ICSS1 and ICSS2). So we > use phandles to differentiate between the two instances. Currently this patch > series adds support for ICSS2 instance on the AM57x SOC. Support for ICSS1 instance > will be added in subsequent patches. Cool, that's an acceptance answer, thanks.