Re: [PATCH] staging: octeon: remove braces from single-line block
Hi!
On 06/02/2021 21:17, Phillip Potter wrote:
> This removes the braces from the if statement that checks the
> physical node return value in cvm_oct_phy_setup_device, as this
> block contains only one statement. Fixes a style warning.
>
> Signed-off-by: Phillip Potter
Reviewed-by: Alexander Sverdlin
> ---
> drivers/staging/octeon/ethernet-mdio.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-mdio.c
> b/drivers/staging/octeon/ethernet-mdio.c
> index 0bf545849b11..b0fd083a5bf2 100644
> --- a/drivers/staging/octeon/ethernet-mdio.c
> +++ b/drivers/staging/octeon/ethernet-mdio.c
> @@ -146,9 +146,8 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
> goto no_phy;
>
> phy_node = of_parse_phandle(priv->of_node, "phy-handle", 0);
> - if (!phy_node && of_phy_is_fixed_link(priv->of_node)) {
> + if (!phy_node && of_phy_is_fixed_link(priv->of_node))
> phy_node = of_node_get(priv->of_node);
> - }
> if (!phy_node)
> goto no_phy;
>
--
Best regards,
Alexander Sverdlin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 net] staging: octeon: Drop on uncorrectable alignment or FCS error
Hello Andrew,
thank you for your review!
On 17/10/2020 23:02, Andrew Lunn wrote:
>> diff --git a/drivers/staging/octeon/ethernet-rx.c
>> b/drivers/staging/octeon/ethernet-rx.c
>> index 2c16230..9ebd665 100644
>> --- a/drivers/staging/octeon/ethernet-rx.c
>> +++ b/drivers/staging/octeon/ethernet-rx.c
>> @@ -69,15 +69,17 @@ static inline int cvm_oct_check_rcv_error(struct
>> cvmx_wqe *work)
>> else
>> port = work->word1.cn38xx.ipprt;
>>
>> -if ((work->word2.snoip.err_code == 10) && (work->word1.len <= 64)) {
>> +if ((work->word2.snoip.err_code == 10) && (work->word1.len <= 64))
> It would be nice to replace all these err_code magic numbers with #defines.
>
> You should also replace 64 with ETH_ZLEN + ETH_FCS_LEN. I also wonder
> if <= should be just < ?
I think all your comments are valid points, but are rather topics
for separate patches. In this one I've addressed one issue: the structure of
ifs and elses is so deeply nested, that it lead to one logic mistake:
broken packets which cannot be corrected are still not dropped.
Even my patch has two changes in one: error correction and style correction,
but I consider this justified because one was a result of another.
>> /*
>> * Ignore length errors on min size packets. Some
>> * equipment incorrectly pads packets to 64+4FCS
>> * instead of 60+4FCS. Note these packets still get
>> * counted as frame errors.
>> */
>> -} else if (work->word2.snoip.err_code == 5 ||
>> - work->word2.snoip.err_code == 7) {
>> +return 0;
>> +
>> +if (work->word2.snoip.err_code == 5 ||
>> +work->word2.snoip.err_code == 7) {
>> /*
>> * We received a packet with either an alignment error
>> * or a FCS error. This may be signalling that we are
>> @@ -108,7 +110,10 @@ static inline int cvm_oct_check_rcv_error(struct
>> cvmx_wqe *work)
>> /* Port received 0xd5 preamble */
>> work->packet_ptr.s.addr += i + 1;
>> work->word1.len -= i + 5;
>> -} else if ((*ptr & 0xf) == 0xd) {
>> +return 0;
>> +}
>> +
>> +if ((*ptr & 0xf) == 0xd) {
> The comments are not so clear what is going on here. Can this
> incorrectly match a destination MAC address of xD:XX:XX:XX:XX:XX.
>
>> /* Port received 0xd preamble */
>> work->packet_ptr.s.addr += i;
>> work->word1.len -= i + 4;
--
Best regards,
Alexander Sverdlin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] stating: octeon: Drop on uncorrectable alignment or FCS error
Hello Dan,
On 09/10/2020 14:24, Dan Carpenter wrote:
> On Fri, Oct 09, 2020 at 11:46:05AM +0200, Alexander A Sverdlin wrote:
>> --- a/drivers/staging/octeon/ethernet-rx.c
>> +++ b/drivers/staging/octeon/ethernet-rx.c
>> @@ -69,14 +69,16 @@ static inline int cvm_oct_check_rcv_error(struct
>> cvmx_wqe *work)
>> else
>> port = work->word1.cn38xx.ipprt;
>>
>> -if ((work->word2.snoip.err_code == 10) && (work->word1.len <= 64)) {
>> +if ((work->word2.snoip.err_code == 10) && (work->word1.len <= 64))
>> /*
>> * Ignore length errors on min size packets. Some
>> * equipment incorrectly pads packets to 64+4FCS
>> * instead of 60+4FCS. Note these packets still get
>> * counted as frame errors.
>> */
>> -} else if (work->word2.snoip.err_code == 5 ||
>> +return 0;
>> +
>> +if (work->word2.snoip.err_code == 5 ||
>> work->word2.snoip.err_code == 7) {
> This line is indented to match the old code and it no longer matches.
> (Please update the whitespace).
thanks to your comment I took a fresh look onto the patch and found a logic
error
in the change. Please ignore the whole patch for now.
>> /*
>> * We received a packet with either an alignment error
--
Best regards,
Alexander Sverdlin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: octeon: Drop on uncorrectable alignment or FCS error
Hello Greg,
On 10/01/2020 13:48, Greg Kroah-Hartman wrote:
> On Wed, Jan 08, 2020 at 05:10:42PM +0100, Alexander X Sverdlin wrote:
>> From: Alexander Sverdlin
>>
>> Currently in case of alignment or FCS error if the packet cannot be
>> corrected it's still not dropped. Report the error properly and drop the
>> packet while making the code around a little bit more readable.
>>
>> Fixes: 80ff0fd3ab ("Staging: Add octeon-ethernet driver files.")
>> Signed-off-by: Alexander Sverdlin
>> ---
>> drivers/staging/octeon/ethernet-rx.c | 18 +-
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> This driver is now deleted from the tree, sorry.
Now that the driver is restored, would you please consider this patch again?
--
Best regards,
Alexander Sverdlin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: octeon: repair "fixed-link" support
Hello Greg, Dave and all,
the below patch is still applicable as-is, would you please re-consider it now,
as the driver has been undeleted?
On 08/01/2020 17:09, Alexander X Sverdlin wrote:
> From: Alexander Sverdlin
>
> The PHYs must be registered once in device probe function, not in device
> open callback because it's only possible to register them once.
>
> Fixes: a25e278020 ("staging: octeon: support fixed-link phys")
> Signed-off-by: Alexander Sverdlin
> ---
> drivers/staging/octeon/ethernet-mdio.c | 6 --
> drivers/staging/octeon/ethernet.c | 11 +++
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-mdio.c
> b/drivers/staging/octeon/ethernet-mdio.c
> index c798672..d81bddf 100644
> --- a/drivers/staging/octeon/ethernet-mdio.c
> +++ b/drivers/staging/octeon/ethernet-mdio.c
> @@ -147,12 +147,6 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
>
> phy_node = of_parse_phandle(priv->of_node, "phy-handle", 0);
> if (!phy_node && of_phy_is_fixed_link(priv->of_node)) {
> - int rc;
> -
> - rc = of_phy_register_fixed_link(priv->of_node);
> - if (rc)
> - return rc;
> -
> phy_node = of_node_get(priv->of_node);
> }
> if (!phy_node)
> diff --git a/drivers/staging/octeon/ethernet.c
> b/drivers/staging/octeon/ethernet.c
> index f42c381..241a1db 100644
> --- a/drivers/staging/octeon/ethernet.c
> +++ b/drivers/staging/octeon/ethernet.c
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -894,6 +895,16 @@ static int cvm_oct_probe(struct platform_device *pdev)
> break;
> }
>
> + if (priv->of_node &&
> + of_phy_is_fixed_link(priv->of_node)) {
> + r = of_phy_register_fixed_link(priv->of_node);
> + if (r) {
> + netdev_err(dev, "Failed to register
> fixed link for interface %d, port %d\n",
> +interface, priv->ipd_port);
> + dev->netdev_ops = NULL;
> + }
> + }
> +
> if (!dev->netdev_ops) {
> free_netdev(dev);
> } else if (register_netdev(dev) < 0) {
>
--
Best regards,
Alexander Sverdlin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/23] mtd: rawnand: plat_nand: Pass a nand_chip object to all platform_nand_ctrl hooks
Hello!
On 17/08/18 18:09, Boris Brezillon wrote:
Let's make the raw NAND API consistent by patching all helpers and
hooks to take a nand_chip object instead of an mtd_info one or
remove the mtd_info object when both are passed.
In order to do that, we first need to update the platform_nand_ctrl
hooks to take a nand_chip object instead of an mtd_info.
We had temporary plat_nand_xxx() wrappers to the do the mtd -> chip
conversion, but those will be dropped when doing the patching nand_chip
hooks to take a nand_chip object.
Signed-off-by: Boris Brezillon
Reviewed-by: Alexander Sverdlin
For the EP93xx parts:
Acked-by: Alexander Sverdlin
---
arch/arm/mach-ep93xx/snappercl15.c | 7 ++--
arch/arm/mach-ep93xx/ts72xx.c | 7 ++--
arch/arm/mach-imx/mach-qong.c | 11 +++
arch/arm/mach-ixp4xx/ixdp425-setup.c| 3 +-
arch/arm/mach-omap1/board-fsample.c | 2 +-
arch/arm/mach-omap1/board-h2.c | 2 +-
arch/arm/mach-omap1/board-h3.c | 2 +-
arch/arm/mach-omap1/board-nand.c| 3 +-
arch/arm/mach-omap1/board-perseus2.c| 2 +-
arch/arm/mach-omap1/common.h| 2 +-
arch/arm/mach-orion5x/ts78xx-setup.c| 18 ---
arch/arm/mach-pxa/balloon3.c| 8 ++---
arch/arm/mach-pxa/em-x270.c | 5 ++-
arch/arm/mach-pxa/palmtx.c | 5 ++-
arch/mips/alchemy/devboards/db1200.c| 5 ++-
arch/mips/alchemy/devboards/db1300.c| 5 ++-
arch/mips/alchemy/devboards/db1550.c| 5 ++-
arch/mips/netlogic/xlr/platform-flash.c | 4 +--
arch/mips/pnx833x/common/platform.c | 3 +-
arch/mips/rb532/devices.c | 5 ++-
arch/sh/boards/mach-migor/setup.c | 6 ++--
drivers/mtd/nand/raw/plat_nand.c| 57 ++---
include/linux/mtd/rawnand.h | 10 +++---
23 files changed, 101 insertions(+), 76 deletions(-)
diff --git a/arch/arm/mach-ep93xx/snappercl15.c
b/arch/arm/mach-ep93xx/snappercl15.c
index 45940c1d7787..aa03ea79c5f5 100644
--- a/arch/arm/mach-ep93xx/snappercl15.c
+++ b/arch/arm/mach-ep93xx/snappercl15.c
@@ -45,10 +45,9 @@
#define NAND_CTRL_ADDR(chip) (chip->IO_ADDR_W + 0x40)
-static void snappercl15_nand_cmd_ctrl(struct mtd_info *mtd, int cmd,
+static void snappercl15_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
unsigned int ctrl)
{
- struct nand_chip *chip = mtd_to_nand(mtd);
static u16 nand_state = SNAPPERCL15_NAND_WPN;
u16 set;
@@ -73,10 +72,8 @@ static void snappercl15_nand_cmd_ctrl(struct mtd_info *mtd, int cmd,
__raw_writew((cmd & 0xff) | nand_state, chip->IO_ADDR_W);
}
-static int snappercl15_nand_dev_ready(struct mtd_info *mtd)
+static int snappercl15_nand_dev_ready(struct nand_chip *chip)
{
- struct nand_chip *chip = mtd_to_nand(mtd);
-
return !!(__raw_readw(NAND_CTRL_ADDR(chip)) & SNAPPERCL15_NAND_RDY);
}
diff --git a/arch/arm/mach-ep93xx/ts72xx.c b/arch/arm/mach-ep93xx/ts72xx.c
index c089a2a4fe30..26259dd9e951 100644
--- a/arch/arm/mach-ep93xx/ts72xx.c
+++ b/arch/arm/mach-ep93xx/ts72xx.c
@@ -76,11 +76,9 @@ static void __init ts72xx_map_io(void)
#define TS72XX_NAND_CONTROL_ADDR_LINE 22 /* 0xN040 */
#define TS72XX_NAND_BUSY_ADDR_LINE23 /* 0xN080 */
-static void ts72xx_nand_hwcontrol(struct mtd_info *mtd,
+static void ts72xx_nand_hwcontrol(struct nand_chip *chip,
int cmd, unsigned int ctrl)
{
- struct nand_chip *chip = mtd_to_nand(mtd);
-
if (ctrl & NAND_CTRL_CHANGE) {
void __iomem *addr = chip->IO_ADDR_R;
unsigned char bits;
@@ -99,9 +97,8 @@ static void ts72xx_nand_hwcontrol(struct mtd_info *mtd,
__raw_writeb(cmd, chip->IO_ADDR_W);
}
-static int ts72xx_nand_device_ready(struct mtd_info *mtd)
+static int ts72xx_nand_device_ready(struct nand_chip *chip)
{
- struct nand_chip *chip = mtd_to_nand(mtd);
void __iomem *addr = chip->IO_ADDR_R;
addr += (1 << TS72XX_NAND_BUSY_ADDR_LINE);
diff --git a/arch/arm/mach-imx/mach-qong.c b/arch/arm/mach-imx/mach-qong.c
index 42a700053103..ff015f603ac9 100644
--- a/arch/arm/mach-imx/mach-qong.c
+++ b/arch/arm/mach-imx/mach-qong.c
@@ -129,10 +129,9 @@ static void qong_init_nor_mtd(void)
/*
* Hardware specific access to control-lines
*/
-static void qong_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int
ctrl)
+static void qong_nand_cmd_ctrl(struct nand_chip *nand_chip, int cmd,
+ unsigned int ctrl)
{
- struct nand_chip *nand_chip = mtd_to_nand(mtd);
-
if (cmd == NAND_CMD_NONE)
return;
@@ -145,14 +144,14 @@ static void qong_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
/*
* Read the Device Ready pin.
*/
-static int qong_nand_devic
