* [PATCH v3 1/2] mtd: nand: fix generating over-boundary ECC data when writing
From: rogercc.lin-NuS5LvNUpcJWk0Htik3J/w @ 2016-08-30 3:02 UTC (permalink / raw)
To: boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
jorge.ramirez-ortiz-QSEj5FYQhm4dnm+yROfE0A,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, RogerCC Lin,
blogic-p3rKhJxN3npAfugRpC6u6w
In-Reply-To: <1472526132-12049-1-git-send-email-rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
From: RogerCC Lin <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
When mtk_ecc_encode() is writing the ECC parity data to the OOB
region,because each register is 4 bytes in length,but the len's unit is
in bytes,the operation in the for loop will cross the ECC's boundary.
Signed-off-by: RogerCC Lin <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
drivers/mtd/nand/mtk_ecc.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index 25a4fbd..495538e 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -366,7 +366,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
u8 *data, u32 bytes)
{
dma_addr_t addr;
- u32 *p, len, i;
+ u8 *p;
+ u32 len, i, val;
int ret = 0;
addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
@@ -392,11 +393,14 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
- p = (u32 *)(data + bytes);
+ p = data + bytes;
/* write the parity bytes generated by the ECC back to the OOB region */
- for (i = 0; i < len; i++)
- p[i] = readl(ecc->regs + ECC_ENCPAR(i));
+ for (i = 0; i < len; i++) {
+ if ((i % 4) == 0)
+ val = readl(ecc->regs + ECC_ENCPAR(i >> 2));
+ p[i] = (val >> ((i % 4) * 8)) & 0xff;
+ }
timeout:
dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 0/2] mtd: nand: fix writing incorrect ECC data in OOB region
From: rogercc.lin-NuS5LvNUpcJWk0Htik3J/w @ 2016-08-30 3:02 UTC (permalink / raw)
To: boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
jorge.ramirez-ortiz-QSEj5FYQhm4dnm+yROfE0A,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, RogerCC Lin,
blogic-p3rKhJxN3npAfugRpC6u6w
From: RogerCC Lin <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
This series fix chances to write incorrect ECC data which may cause uncorrectable ECC error when reading.
changes since v2:
- use macro for shift operation.
changes since v1:
- separate patches into 2.
- use shift operator with byte access to avoid endianness conversion problems.
- follow linux coding style.
The patch passed the test of UBIFS file-system read/write on Mediatek's RFB. The tested driver is checked-out from LEDE OpenWRT project's upstream driver, which is pretty much same as nand/next branch upstream driver(git clone https://git.lede-project.org/source.git).
RogerCC Lin (2):
mtd: nand: fix generating over-boundary ECC data when writing.
mtd: nand: fix chances to create incomplete ECC data when writing.
drivers/mtd/nand/mtk_ecc.c | 12 ++++++++----
drivers/mtd/nand/mtk_nand.c | 9 +++++----
2 files changed, 13 insertions(+), 8 deletions(-)
^ permalink raw reply
* [PATCH net v3 9/9] net: ethernet: mediatek: fix error handling inside mtk_mdio_init
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
Return -ENODEV if the MDIO bus is disabled in the device tree.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Acked-by: John Crispin <john@phrozen.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 2fc48bb..dfc12ab 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -304,7 +304,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
}
if (!of_device_is_available(mii_np)) {
- ret = 0;
+ ret = -ENODEV;
goto err_put_node;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 8/9] net: ethernet: mediatek: use devm_mdiobus_alloc instead of mdiobus_alloc inside mtk_mdio_init
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
a lot of parts in the driver uses devm_* APIs to gain benefits from the
device resource management, so devm_mdiobus_alloc is also used instead
of mdiobus_alloc to have more elegant code flow.
Using common code provided by the devm_* helps to
1) have simplified the code flow as [1] says
2) decrease the risk of incorrect error handling by human
3) only a few drivers used it since it was proposed on linux 3.16,
so just hope to promote for this.
Ref:
[1] https://patchwork.ozlabs.org/patch/344093/
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 23 ++++++-----------------
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index f77e173..2fc48bb 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -295,7 +295,7 @@ err_phy:
static int mtk_mdio_init(struct mtk_eth *eth)
{
struct device_node *mii_np;
- int err;
+ int ret;
mii_np = of_get_child_by_name(eth->dev->of_node, "mdio-bus");
if (!mii_np) {
@@ -304,13 +304,13 @@ static int mtk_mdio_init(struct mtk_eth *eth)
}
if (!of_device_is_available(mii_np)) {
- err = 0;
+ ret = 0;
goto err_put_node;
}
- eth->mii_bus = mdiobus_alloc();
+ eth->mii_bus = devm_mdiobus_alloc(eth->dev);
if (!eth->mii_bus) {
- err = -ENOMEM;
+ ret = -ENOMEM;
goto err_put_node;
}
@@ -321,20 +321,11 @@ static int mtk_mdio_init(struct mtk_eth *eth)
eth->mii_bus->parent = eth->dev;
snprintf(eth->mii_bus->id, MII_BUS_ID_SIZE, "%s", mii_np->name);
- err = of_mdiobus_register(eth->mii_bus, mii_np);
- if (err)
- goto err_free_bus;
- of_node_put(mii_np);
-
- return 0;
-
-err_free_bus:
- mdiobus_free(eth->mii_bus);
+ ret = of_mdiobus_register(eth->mii_bus, mii_np);
err_put_node:
of_node_put(mii_np);
- eth->mii_bus = NULL;
- return err;
+ return ret;
}
static void mtk_mdio_cleanup(struct mtk_eth *eth)
@@ -343,8 +334,6 @@ static void mtk_mdio_cleanup(struct mtk_eth *eth)
return;
mdiobus_unregister(eth->mii_bus);
- of_node_put(eth->mii_bus->dev.of_node);
- mdiobus_free(eth->mii_bus);
}
static inline void mtk_irq_disable(struct mtk_eth *eth, u32 mask)
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 7/9] net: ethernet: mediatek: fix the missing of_node_put() after node is used done inside mtk_mdio_init
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
This patch adds the missing of_node_put() after finishing the usage
of of_get_child_by_name.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Acked-by: John Crispin <john@phrozen.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8dcd008..f77e173 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -324,6 +324,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
err = of_mdiobus_register(eth->mii_bus, mii_np);
if (err)
goto err_free_bus;
+ of_node_put(mii_np);
return 0;
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 6/9] net: ethernet: mediatek: fix issue of driver removal with interface is up
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
mtk_stop() must be called to stop for freeing DMA
resources acquired and restoring state changed by mtk_open()
firstly when module removal.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 5bfca65..8dcd008 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1903,6 +1903,14 @@ err_free_dev:
static int mtk_remove(struct platform_device *pdev)
{
struct mtk_eth *eth = platform_get_drvdata(pdev);
+ int i;
+
+ /* stop all devices to make sure that dma is properly shut down */
+ for (i = 0; i < MTK_MAC_COUNT; i++) {
+ if (!eth->netdev[i])
+ continue;
+ mtk_stop(eth->netdev[i]);
+ }
clk_disable_unprepare(eth->clks[MTK_CLK_ETHIF]);
clk_disable_unprepare(eth->clks[MTK_CLK_ESW]);
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 5/9] net: ethernet: mediatek: fix logic unbalance between probe and remove
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
original mdio_cleanup is not in the symmetric place against where
mdio_init is, so relocate mdio_cleanup to the right one.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Acked-by: John Crispin <john@phrozen.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8d87748..5bfca65 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1508,7 +1508,6 @@ static void mtk_uninit(struct net_device *dev)
struct mtk_eth *eth = mac->hw;
phy_disconnect(mac->phy_dev);
- mtk_mdio_cleanup(eth);
mtk_irq_disable(eth, ~0);
}
@@ -1913,6 +1912,7 @@ static int mtk_remove(struct platform_device *pdev)
netif_napi_del(ð->tx_napi);
netif_napi_del(ð->rx_napi);
mtk_cleanup(eth);
+ mtk_mdio_cleanup(eth);
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 4/9] net: ethernet: mediatek: remove redundant free_irq for devm_request_irq allocated irq
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
these irqs are not used for shared irq and disabled during ethernet stops.
irq requested by devm_request_irq is safe to be freed automatically on
driver detach.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Acked-by: John Crispin <john@phrozen.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 0f0072e..8d87748 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1510,8 +1510,6 @@ static void mtk_uninit(struct net_device *dev)
phy_disconnect(mac->phy_dev);
mtk_mdio_cleanup(eth);
mtk_irq_disable(eth, ~0);
- free_irq(eth->irq[1], dev);
- free_irq(eth->irq[2], dev);
}
static int mtk_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 3/9] net: ethernet: mediatek: fix API usage with skb_free_frag
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
use skb_free_frag() instead of legacy put_page()
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Acked-by: John Crispin <john@phrozen.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 33a890c..0f0072e 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -868,7 +868,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
/* receive data */
skb = build_skb(data, ring->frag_size);
if (unlikely(!skb)) {
- put_page(virt_to_head_page(new_data));
+ skb_free_frag(new_data);
netdev->stats.rx_dropped++;
goto release_desc;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 2/9] net: ethernet: mediatek: fix incorrect return value of devm_clk_get with EPROBE_DEFER
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
1) If the return value of devm_clk_get is EPROBE_DEFER, we should
defer probing the driver. The change is verified and works based
on 4.8-rc1 staying with the latest clk-next code for MT7623.
2) Changing with the usage of loops to work out if all clocks
required are fine
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 39 ++++++++++++++++-------------
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 22 ++++++++++------
2 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6e4a6ca..33a890c 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -50,6 +50,10 @@ static const struct mtk_ethtool_stats {
MTK_ETHTOOL_STAT(rx_flow_control_packets),
};
+static const char * const mtk_clks_source_name[] = {
+ "ethif", "esw", "gp1", "gp2"
+};
+
void mtk_w32(struct mtk_eth *eth, u32 val, unsigned reg)
{
__raw_writel(val, eth->base + reg);
@@ -1811,6 +1815,7 @@ static int mtk_probe(struct platform_device *pdev)
if (!eth)
return -ENOMEM;
+ eth->dev = &pdev->dev;
eth->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(eth->base))
return PTR_ERR(eth->base);
@@ -1845,21 +1850,21 @@ static int mtk_probe(struct platform_device *pdev)
return -ENXIO;
}
}
+ for (i = 0; i < ARRAY_SIZE(eth->clks); i++) {
+ eth->clks[i] = devm_clk_get(eth->dev,
+ mtk_clks_source_name[i]);
+ if (IS_ERR(eth->clks[i])) {
+ if (PTR_ERR(eth->clks[i]) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ return -ENODEV;
+ }
+ }
- eth->clk_ethif = devm_clk_get(&pdev->dev, "ethif");
- eth->clk_esw = devm_clk_get(&pdev->dev, "esw");
- eth->clk_gp1 = devm_clk_get(&pdev->dev, "gp1");
- eth->clk_gp2 = devm_clk_get(&pdev->dev, "gp2");
- if (IS_ERR(eth->clk_esw) || IS_ERR(eth->clk_gp1) ||
- IS_ERR(eth->clk_gp2) || IS_ERR(eth->clk_ethif))
- return -ENODEV;
-
- clk_prepare_enable(eth->clk_ethif);
- clk_prepare_enable(eth->clk_esw);
- clk_prepare_enable(eth->clk_gp1);
- clk_prepare_enable(eth->clk_gp2);
+ clk_prepare_enable(eth->clks[MTK_CLK_ETHIF]);
+ clk_prepare_enable(eth->clks[MTK_CLK_ESW]);
+ clk_prepare_enable(eth->clks[MTK_CLK_GP1]);
+ clk_prepare_enable(eth->clks[MTK_CLK_GP2]);
- eth->dev = &pdev->dev;
eth->msg_enable = netif_msg_init(mtk_msg_level, MTK_DEFAULT_MSG_ENABLE);
INIT_WORK(ð->pending_work, mtk_pending_work);
@@ -1902,10 +1907,10 @@ static int mtk_remove(struct platform_device *pdev)
{
struct mtk_eth *eth = platform_get_drvdata(pdev);
- clk_disable_unprepare(eth->clk_ethif);
- clk_disable_unprepare(eth->clk_esw);
- clk_disable_unprepare(eth->clk_gp1);
- clk_disable_unprepare(eth->clk_gp2);
+ clk_disable_unprepare(eth->clks[MTK_CLK_ETHIF]);
+ clk_disable_unprepare(eth->clks[MTK_CLK_ESW]);
+ clk_disable_unprepare(eth->clks[MTK_CLK_GP1]);
+ clk_disable_unprepare(eth->clks[MTK_CLK_GP2]);
netif_napi_del(ð->tx_napi);
netif_napi_del(ð->rx_napi);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index f82e3ac..6e1ade7 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -290,6 +290,17 @@ enum mtk_tx_flags {
MTK_TX_FLAGS_PAGE0 = 0x02,
};
+/* This enum allows us to identify how the clock is defined on the array of the
+ * clock in the order
+ */
+enum mtk_clks_map {
+ MTK_CLK_ETHIF,
+ MTK_CLK_ESW,
+ MTK_CLK_GP1,
+ MTK_CLK_GP2,
+ MTK_CLK_MAX
+};
+
/* struct mtk_tx_buf - This struct holds the pointers to the memory pointed at
* by the TX descriptor s
* @skb: The SKB pointer of the packet being sent
@@ -370,10 +381,7 @@ struct mtk_rx_ring {
* @scratch_ring: Newer SoCs need memory for a second HW managed TX ring
* @phy_scratch_ring: physical address of scratch_ring
* @scratch_head: The scratch memory that scratch_ring points to.
- * @clk_ethif: The ethif clock
- * @clk_esw: The switch clock
- * @clk_gp1: The gmac1 clock
- * @clk_gp2: The gmac2 clock
+ * @clks: clock array for all clocks required
* @mii_bus: If there is a bus we need to create an instance for it
* @pending_work: The workqueue used to reset the dma ring
*/
@@ -400,10 +408,8 @@ struct mtk_eth {
struct mtk_tx_dma *scratch_ring;
dma_addr_t phy_scratch_ring;
void *scratch_head;
- struct clk *clk_ethif;
- struct clk *clk_esw;
- struct clk *clk_gp1;
- struct clk *clk_gp2;
+ struct clk *clks[MTK_CLK_MAX];
+
struct mii_bus *mii_bus;
struct work_struct pending_work;
};
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 1/9] net: ethernet: mediatek: fix fails from TX housekeeping due to incorrect port setup
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
In-Reply-To: <1472525965-10661-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
which net device the SKB is complete for depends on the forward port
on txd4 on the corresponding TX descriptor, but the information isn't
set up well in case of SKB fragments that would lead to watchdog timeout
from the upper layer, so fix it up.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Acked-by: John Crispin <john@phrozen.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 1801fd8..6e4a6ca 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -587,14 +587,15 @@ static int mtk_tx_map(struct sk_buff *skb, struct net_device *dev,
dma_addr_t mapped_addr;
unsigned int nr_frags;
int i, n_desc = 1;
- u32 txd4 = 0;
+ u32 txd4 = 0, fport;
itxd = ring->next_free;
if (itxd == ring->last_free)
return -ENOMEM;
/* set the forward port */
- txd4 |= (mac->id + 1) << TX_DMA_FPORT_SHIFT;
+ fport = (mac->id + 1) << TX_DMA_FPORT_SHIFT;
+ txd4 |= fport;
tx_buf = mtk_desc_to_tx_buf(ring, itxd);
memset(tx_buf, 0, sizeof(*tx_buf));
@@ -652,7 +653,7 @@ static int mtk_tx_map(struct sk_buff *skb, struct net_device *dev,
WRITE_ONCE(txd->txd3, (TX_DMA_SWC |
TX_DMA_PLEN0(frag_map_size) |
last_frag * TX_DMA_LS0));
- WRITE_ONCE(txd->txd4, 0);
+ WRITE_ONCE(txd->txd4, fport);
tx_buf->skb = (struct sk_buff *)MTK_DMA_DUMMY_DESC;
tx_buf = mtk_desc_to_tx_buf(ring, txd);
--
1.9.1
^ permalink raw reply related
* [PATCH net v3 0/9] net: ethernet: mediatek: a couple of fixes
From: sean.wang @ 2016-08-30 2:59 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, Sean Wang
From: Sean Wang <sean.wang@mediatek.com>
a couple of fixes come out from integrating with linux-4.8 rc1
they all are verified and workable on linux-4.8 rc1
Changes since v1:
- usage of loops to work out if all required clock are ready instead
of tedious coding
- remove redundant pinctrl setup that is already done by core driver
thanks for careful and patient reviewing by Andrew Lunn
- splitting distinct changes into the separate patches
- change variable naming from err to ret for readable coding
Changes since v2:
- restore to original clock disabling sequence that is changed
accidentally in the last version
- refine the commit log that would cause misunderstanding what has
been done in the changes
- refine the commit log that would cause footnote losing due to
improper delimiter use
Sean Wang (9):
net: ethernet: mediatek: fix fails from TX housekeeping due to
incorrect port setup
net: ethernet: mediatek: fix incorrect return value of devm_clk_get
with EPROBE_DEFER
net: ethernet: mediatek: fix API usage with skb_free_frag
net: ethernet: mediatek: remove redundant free_irq for
devm_request_irq allocated irq
net: ethernet: mediatek: fix logic unbalance between probe and remove
net: ethernet: mediatek: fix issue of driver removal with interface is
up
net: ethernet: mediatek: fix the missing of_node_put() after node is
used done inside mtk_mdio_init
net: ethernet: mediatek: use devm_mdiobus_alloc instead of
mdiobus_alloc inside mtk_mdio_init
net: ethernet: mediatek: fix error handling inside mtk_mdio_init
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 82 +++++++++++++++--------------
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 22 +++++---
2 files changed, 56 insertions(+), 48 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH -next] iio: adc: mt2701: Remove redundant dev_err call in mt6577_auxadc_probe()
From: zhiyong tao @ 2016-08-30 1:29 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Wei Yongjun, linux-iio-u79uwXL29TY76Z2rM5mHXA,
zhiyong.tao-NuS5LvNUpcJWk0Htik3J/w,
erin.lo-NuS5LvNUpcJWk0Htik3J/w,
liguo.zhang-NuS5LvNUpcJWk0Htik3J/w,
yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w, Wei Yongjun,
Peter Meerwald-Stadler, Hartmut Knaack, Matthias Brugger,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
eddie.huang-NuS5LvNUpcJWk0Htik3J/w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <88307772-dd0f-a2f2-efd8-6c64a0cb14e4-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Mon, 2016-08-29 at 19:47 +0100, Jonathan Cameron wrote:
> On 26/08/16 15:26, Wei Yongjun wrote:
> > From: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> >
> > There is a error message within devm_ioremap_resource
> > already, so remove the dev_err call to avoid redundant
> > error message.
> >
> > Signed-off-by: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Hi Wei,
>
> I'm not so sure these are actually redundant. Overly verbose perhaps,
> but they do provide additional information in the log as the
> devm_ioremap_resource doesn't know 'what' has failed, merely how
> it has failed.
>
> I'm happy to take these, if the driver author / maintainers
> agree, but I'm not keen enough to take them without that.
>
> Zhiyong Tao, what do you think on this one?
>
> Jonathan
Dear Jonathan,
Thank you for your suggestion. I think that we can know 'what' has
failed in the driver, if we retain the error message log. It maybe
advantageous to debug issues.
Zhiyong
> > ---
> > drivers/iio/adc/mt6577_auxadc.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> > index 2d104c8..a268954 100644
> > --- a/drivers/iio/adc/mt6577_auxadc.c
> > +++ b/drivers/iio/adc/mt6577_auxadc.c
> > @@ -206,10 +206,8 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > adc_dev->reg_base = devm_ioremap_resource(&pdev->dev, res);
> > - if (IS_ERR(adc_dev->reg_base)) {
> > - dev_err(&pdev->dev, "failed to get auxadc base address\n");
> > + if (IS_ERR(adc_dev->reg_base))
> > return PTR_ERR(adc_dev->reg_base);
> > - }
> >
> > adc_dev->adc_clk = devm_clk_get(&pdev->dev, "main");
> > if (IS_ERR(adc_dev->adc_clk)) {
> >
> >
> >
>
^ permalink raw reply
* Re: [PATCH -next] iio: adc: mt2701: Remove redundant dev_err call in mt6577_auxadc_probe()
From: Jonathan Cameron @ 2016-08-29 18:47 UTC (permalink / raw)
To: Wei Yongjun, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Matthias Brugger, Zhiyong Tao
Cc: Wei Yongjun, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1472221581-28836-1-git-send-email-weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 26/08/16 15:26, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> There is a error message within devm_ioremap_resource
> already, so remove the dev_err call to avoid redundant
> error message.
>
> Signed-off-by: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Hi Wei,
I'm not so sure these are actually redundant. Overly verbose perhaps,
but they do provide additional information in the log as the
devm_ioremap_resource doesn't know 'what' has failed, merely how
it has failed.
I'm happy to take these, if the driver author / maintainers
agree, but I'm not keen enough to take them without that.
Zhiyong Tao, what do you think on this one?
Jonathan
> ---
> drivers/iio/adc/mt6577_auxadc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> index 2d104c8..a268954 100644
> --- a/drivers/iio/adc/mt6577_auxadc.c
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -206,10 +206,8 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> adc_dev->reg_base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(adc_dev->reg_base)) {
> - dev_err(&pdev->dev, "failed to get auxadc base address\n");
> + if (IS_ERR(adc_dev->reg_base))
> return PTR_ERR(adc_dev->reg_base);
> - }
>
> adc_dev->adc_clk = devm_clk_get(&pdev->dev, "main");
> if (IS_ERR(adc_dev->adc_clk)) {
>
>
>
^ permalink raw reply
* Re: [PATCH v12 1/4] clk: mediatek: Add MT2701 clock support
From: Stephen Boyd @ 2016-08-29 17:46 UTC (permalink / raw)
To: James Liao
Cc: Erin Lo, Matthias Brugger, Mike Turquette, Rob Herring,
Arnd Bergmann, Sascha Hauer, Daniel Kurtz, Philipp Zabel,
devicetree, linux-arm-kernel, linux-kernel, linux-mediatek,
linux-clk, srv_heupstream, Shunli Wang
In-Reply-To: <1472271377.21203.12.camel@mtksdaap41>
On 08/27, James Liao wrote:
> On Wed, 2016-08-24 at 10:49 -0700, Stephen Boyd wrote:
> > On 08/22, Erin Lo wrote:
> > > +
> > > +static void __init mtk_infrasys_init_early(struct device_node *node)
> > > +{
> > > + int r, i;
> > > +
> > > + if (!infra_clk_data) {
> > > + infra_clk_data = mtk_alloc_clk_data(CLK_INFRA_NR);
> > > +
> > > + for (i = 0; i < CLK_INFRA_NR; i++)
> > > + infra_clk_data->clks[i] = ERR_PTR(-EPROBE_DEFER);
> > > + }
> > > +
> > > + mtk_clk_register_factors(infra_fixed_divs, ARRAY_SIZE(infra_fixed_divs),
> > > + infra_clk_data);
> > > +
> > > + r = of_clk_add_provider(node, of_clk_src_onecell_get, infra_clk_data);
> > > + if (r)
> > > + pr_err("%s(): could not register clock provider: %d\n",
> > > + __func__, r);
> > > +}
> > > +CLK_OF_DECLARE(mtk_infra, "mediatek,mt2701-infracfg", mtk_infrasys_init_early);
> >
> > This should use CLK_OF_DECLARE_DRIVER? Has this been tested on
> > latest clk-next? Some recent patches make it so that
> > CLK_OF_DECLARE() prevents platform devices from being created for
> > the associated DT nodes that match during of_clk_init().
>
> Oops, you are right. Clocks in infra_clks are gone on clk-next, but they
> are good on v4.8-rc1.
>
> I register clk13m in infra_fixed_divs through CLK_OF_DECLARE() so that
> it can be registered as early as possible because it will be referred by
> the timer driver. Is there a formal way to separate clock registrations
> on the same clock provider? Or should I move infra_clks registration
> into CLK_OF_DECLARE()?
The way to do this is use CLK_OF_DECLARE_DRIVER() and then do the
early clks in the CLK_OF callback and the rest of them in the
driver probe.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH net v2 9/9] net: ethernet: mediatek: fix error handling inside mtk_mdio_init
From: Andrew Lunn @ 2016-08-29 16:36 UTC (permalink / raw)
To: sean.wang; +Cc: john, davem, nbd, netdev, linux-mediatek, keyhaede
In-Reply-To: <1472447003-30726-10-git-send-email-sean.wang@mediatek.com>
On Mon, Aug 29, 2016 at 01:03:23PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> return -ENODEV if no child is found in MDIO bus.
The "no child" is wrong here, and got me confused. What the code is
actually doing is of_device_is_available() which is looking to see if
there is a status = "okay". So what the change log should say is:
Return -ENODEV if the MDIO bus is disabled in the device tree.
Once that has been corrected:
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Acked-by: John Crispin <john@phrozen.org>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index f741c6a..e48b2a4 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -304,7 +304,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
> }
>
> if (!of_device_is_available(mii_np)) {
> - ret = 0;
> + ret = -ENODEV;
> goto err_put_node;
> }
>
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH net v2 9/9] net: ethernet: mediatek: fix error handling inside mtk_mdio_init
From: Sean Wang @ 2016-08-29 16:10 UTC (permalink / raw)
To: andrew; +Cc: john, davem, nbd, netdev, linux-mediatek, keyhaede
In-Reply-To: <20160829131558.GE12465@lunn.ch>
Date: Mon, 29 Aug 2016 15:15:58 +0200,Andrew Lunn wrote:
>On Mon, Aug 29, 2016 at 01:03:23PM +0800, sean.wang@mediatek.com wrote:
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> return -ENODEV if no child is found in MDIO bus.
>
>Hi Sean
>
>Why is it an error not to have any children on the bus?
>
>Say i have a fibre optical module connected to the MAC. It is unlikely
>to have an MII interface, so i would not list it on the bus. With this
>change, if i have the mdio-bus node in my device tree, i don't get a
>working system. Without this change, it simply does not instantiate
>the MDIO device, and returns without an error.
>
>I think this patch should be dropped, or maybe a comment adding, why
>the current code returns 0 at this point.
>
> Andrew
>
Hi Andrew,
Sorry, i didn't add the comment enough and well on the patch for let you can't see
what i am done
the patch I just want to let driver know if device tree is defined well at the earlier time
although original driver still works without this patch
the original logic on the driver is
1) returning -ENODEV if no mdio_bus defined in the device tree.
2) returning -ENODEV inside ndo_init callback if no phy_dev detected on the bus
--
after with the patch, the logic becomes
1) returning -ENODEV if no mdio_bus defined in the device tree
2) returning -ENODEV if no phy_dev defined in the device tree
a. that is used to check if the device tree about phy_dev is defined well
b. and it could be aligned with the above 1) is doing
3) returning -ENODEV inside ndo_init callback if no phy device detected on the bus
(that is used to test if phy_device is workable or encounters real phy problems)
so add 2) help to make distinguish if it is a device tree definition
problem or a real phy problem at the earlier time
this is my whole thought
thanks,
Sean
>>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> Acked-by: John Crispin <john@phrozen.org>
>> ---
>> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> index f741c6a..e48b2a4 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -304,7 +304,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
>> }
>>
>> if (!of_device_is_available(mii_np)) {
>> - ret = 0;
>> + ret = -ENODEV;
>> goto err_put_node;
>> }
>>
>> --
>> 1.9.1
>
^ permalink raw reply
* Re: [PATCH net v2 9/9] net: ethernet: mediatek: fix error handling inside mtk_mdio_init
From: Andrew Lunn @ 2016-08-29 13:15 UTC (permalink / raw)
To: sean.wang; +Cc: john, davem, nbd, netdev, linux-mediatek, keyhaede
In-Reply-To: <1472447003-30726-10-git-send-email-sean.wang@mediatek.com>
On Mon, Aug 29, 2016 at 01:03:23PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> return -ENODEV if no child is found in MDIO bus.
Hi Sean
Why is it an error not to have any children on the bus?
Say i have a fibre optical module connected to the MAC. It is unlikely
to have an MII interface, so i would not list it on the bus. With this
change, if i have the mdio-bus node in my device tree, i don't get a
working system. Without this change, it simply does not instantiate
the MDIO device, and returns without an error.
I think this patch should be dropped, or maybe a comment adding, why
the current code returns 0 at this point.
Andrew
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Acked-by: John Crispin <john@phrozen.org>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index f741c6a..e48b2a4 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -304,7 +304,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
> }
>
> if (!of_device_is_available(mii_np)) {
> - ret = 0;
> + ret = -ENODEV;
> goto err_put_node;
> }
>
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH net v2 8/9] net: ethernet: mediatek: use devm_mdiobus_alloc instead of mdiobus_alloc inside mtk_mdio_init
From: Andrew Lunn @ 2016-08-29 13:06 UTC (permalink / raw)
To: sean.wang; +Cc: john, davem, nbd, netdev, linux-mediatek, keyhaede
In-Reply-To: <1472447003-30726-9-git-send-email-sean.wang@mediatek.com>
On Mon, Aug 29, 2016 at 01:03:22PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> a lot of parts in the driver uses devm_* APIs to gain benefits from the
> device resource management, so devm_mdiobus_alloc is also used instead
> of mdiobus_alloc to have more elegant code flow.
>
> Using common code provided by the devm_* helps to
> 1) have simplified the code flow as [1] says
> 2) decrease the risk of incorrect error handling by human
> 3) only a few drivers used it since it ware proposed on linux 3.16,
> so just hope to promote for this.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>
> ---
> ref.
> [1] https://patchwork.ozlabs.org/patch/344093/
> ---
Hi Sean
Your footnote is below the ---, so won't make it into the change log.
Apart from that,
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 0/2] mtd: nand: fix writing incorrect ECC parity data in OOB region.
From: RogerCC.Lin @ 2016-08-29 11:53 UTC (permalink / raw)
To: Boris Brezillon
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jorge Ramirez-Ortiz,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, blogic-p3rKhJxN3npAfugRpC6u6w
In-Reply-To: <1472470803.11041.4.camel@mtkswgap22>
Hi Boris and all,
I am sorry that though I have change the sender name in the email, but
It seems the patchwork still show the wrong name, I will check what
wrong with my email, sorry for the inconvenience.
Roger
^ permalink raw reply
* [PATCH v2 2/2] mtd: nand: fix chances to create incomplete ECC data when writing.
From: RogerCC.Lin @ 2016-08-29 11:40 UTC (permalink / raw)
To: Boris Brezillon
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jorge Ramirez-Ortiz,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, blogic-p3rKhJxN3npAfugRpC6u6w
In-Reply-To: <20160829101006.2b200e0a@bbrezillon>
When mtk_nfc_do_write_page() comparing the sector number,because the
sector number field is at the 12th-bit position of NFI_BYTELEN
register,the masked register should be shifted 12 bits before being
compared.The result of this bug may cause the second subpage has
incomplete ECC parity bytes.
Signed-off-by: RogerCC Lin <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
drivers/mtd/nand/mtk_nand.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/mtk_nand.c b/drivers/mtd/nand/mtk_nand.c
index ddaa2ac..5e0727c 100644
--- a/drivers/mtd/nand/mtk_nand.c
+++ b/drivers/mtd/nand/mtk_nand.c
@@ -699,8 +699,8 @@ static int mtk_nfc_do_write_page(struct mtd_info
*mtd, struct nand_chip *chip,
}
ret = readl_poll_timeout_atomic(nfc->regs + NFI_ADDRCNTR, reg,
- (reg & CNTR_MASK) >= chip->ecc.steps,
- 10, MTK_TIMEOUT);
+ ((reg & CNTR_MASK) >> 12) >= chip->ecc.steps,
+ 10, MTK_TIMEOUT);
if (ret)
dev_err(dev, "hwecc write timeout\n");
@@ -902,8 +902,8 @@ static int mtk_nfc_read_subpage(struct mtd_info
*mtd, struct nand_chip *chip,
dev_warn(nfc->dev, "read ahb/dma done timeout\n");
rc = readl_poll_timeout_atomic(nfc->regs + NFI_BYTELEN, reg,
- (reg & CNTR_MASK) >= sectors, 10,
- MTK_TIMEOUT);
+ ((reg & CNTR_MASK) >> 12) >= sectors,
+ 10, MTK_TIMEOUT);
if (rc < 0) {
dev_err(nfc->dev, "subpage done timeout\n");
bitflips = -EIO;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 1/2] mtd: nand: fix generating over-boundary ECC data when writing.
From: RogerCC.Lin @ 2016-08-29 11:40 UTC (permalink / raw)
To: Boris Brezillon
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jorge Ramirez-Ortiz,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, blogic-p3rKhJxN3npAfugRpC6u6w
In-Reply-To: <20160829101006.2b200e0a@bbrezillon>
When mtk_ecc_encode() is writing the ECC parity data to the OOB
region,because each register is 4 bytes in length,but the len's unit is
in bytes,the operation in the for loop will cross the ECC's boundary.
Signed-off-by: RogerCC Lin <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
drivers/mtd/nand/mtk_ecc.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index 25a4fbd..495538e 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -366,7 +366,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct
mtk_ecc_config *config,
u8 *data, u32 bytes)
{
dma_addr_t addr;
- u32 *p, len, i;
+ u8 *p;
+ u32 len, i, val;
int ret = 0;
addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE); @@
-392,11 +393,14 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct
mtk_ecc_config *config,
/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
- p = (u32 *)(data + bytes);
+ p = data + bytes;
/* write the parity bytes generated by the ECC back to the OOB region
*/
- for (i = 0; i < len; i++)
- p[i] = readl(ecc->regs + ECC_ENCPAR(i));
+ for (i = 0; i < len; i++) {
+ if ((i % 4) == 0)
+ val = readl(ecc->regs + ECC_ENCPAR(i >> 2));
+ p[i] = (val >> ((i % 4) * 8)) & 0xff;
+ }
timeout:
dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 0/2] mtd: nand: fix writing incorrect ECC parity data in OOB region.
From: RogerCC.Lin @ 2016-08-29 11:40 UTC (permalink / raw)
To: Boris Brezillon
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jorge Ramirez-Ortiz,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, blogic-p3rKhJxN3npAfugRpC6u6w
In-Reply-To: <20160829101006.2b200e0a@bbrezillon>
This series fix chances to write incorrect ECC data which may cause
uncorrectable ECC error when reading.
changes since v1:
- separate patches into 2.
- use shift operator with byte access to avoid endianness conversion
problems.
- follow linux coding style.
The patch passed the test of UBIFS file-system read/write on Mediatek's
RFB. The tested driver is checked-out from LEDE OpenWRT project's
upstream driver, which is pretty much same as nand/next branch upstream
driver(git clone https://git.lede-project.org/source.git).
RogerCC Lin (2):
mtd: nand: fix generating over-boundary ECC data when writing.
mtd: nand: fix chances to create incomplete ECC data when writing.
drivers/mtd/nand/mtk_ecc.c | 12 ++++++++----
drivers/mtd/nand/mtk_nand.c | 8 ++++----
2 files changed, 12 insertions(+), 8 deletions(-)
^ permalink raw reply
* Re: [Patch] mtd: fix writing incorrect ECC parity data in OOB region.
From: Jorge Ramirez @ 2016-08-29 8:56 UTC (permalink / raw)
To: Boris Brezillon, mtk04561
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, blogic-p3rKhJxN3npAfugRpC6u6w
In-Reply-To: <20160829101006.2b200e0a@bbrezillon>
On 08/29/2016 10:10 AM, Boris Brezillon wrote:
> +Jorge
Thanks Boris. Xiaolei is looking into this - he is aware of the patch.
^ permalink raw reply
* Re: [Patch] mtd: fix writing incorrect ECC parity data in OOB region.
From: Boris Brezillon @ 2016-08-29 8:10 UTC (permalink / raw)
To: mtk04561
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
steven.liu-NuS5LvNUpcJWk0Htik3J/w,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jorge Ramirez-Ortiz,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ, blogic-p3rKhJxN3npAfugRpC6u6w
In-Reply-To: <1472443093.27061.4.camel@mtkswgap22>
+Jorge
Hi Roger,
On Mon, 29 Aug 2016 11:58:13 +0800
mtk04561 <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> Hi Boris,
>
> I found a problem in current Mediatek upstream NAND driver (of nand/next
> branch). When testing the upstream driver with UBIFS file system, it has
> some chance to create incorrect ECC data in the second subpage, this
> makes UBIFS failed to mount. Below patch fixes this problem. Please help
> to review the patch and upstream. Thank you.
>
> Best regards,
> Roger
Can you put this message after the '---' (below your SoB), so that it
does not appear when applying the patch.
>
>
>
> Description:
Drop the 'Description:' line.
> When mtk_ecc_encode() is writing the ECC parity data to the OOB region,
> because each register is 4 bytes in length, but the len's unit is in
> bytes, the operation in the for loop will cross the ECC's boundary.
>
> And when mtk_nfc_do_write_page() comparing the sector number, because
> the sector number field is at the 12th-bit position of NFI_BYTELEN
> register, the masked register should be shifted 12 bits before being
> compared. The result of this bug may cause the second subpage has
> incomplete ECC parity bytes.
Maybe you could split those changes in 2 patches (not a strong
requirement).
>
> Test:
Drop the 'Test:' line.
> The patch passed the test of UBIFS file-system read/write on Mediatek's
> RFB. The tested driver is checked-out from LEDE OpenWRT project's
> upstream driver, which is pretty much same as nand/next branch upstream
> driver(git clone https://git.lede-project.org/source.git).
>
>
> Signed-off-by: RogerCC Lin <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> index 25a4fbd..0a7ce8d 100644
> --- a/drivers/mtd/nand/mtk_ecc.c
> +++ b/drivers/mtd/nand/mtk_ecc.c
> @@ -366,7 +366,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct
> mtk_ecc_config *config,
> u8 *data, u32 bytes)
> {
> dma_addr_t addr;
> - u32 *p, len, i;
> + u8 *p;
> + u32 len, i, val;
> int ret = 0;
>
> addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> @@ -395,8 +396,11 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct
> mtk_ecc_config *config,
> p = (u32 *)(data + bytes);
Hm, you should now do:
p = data + bytes;
>
> /* write the parity bytes generated by the ECC back to the OOB
> region */
> - for (i = 0; i < len; i++)
> - p[i] = readl(ecc->regs + ECC_ENCPAR(i));
> + for (i = 0; i < len; i++) {
> + if ((i % 4) == 0)
> + val = readl(ecc->regs + ECC_ENCPAR(i >> 2));
> + p[i] = *((u8 *)&val + (i % 4));
Please, use a shift operator here:
p[i] = (val >> ((i % 4) * 8)) & 0xff;
Otherwise you're exposed to endianness conversion problems (this works
fine in your case because you're using a little-endian kernel).
> + }
> timeout:
>
> dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
> diff --git a/drivers/mtd/nand/mtk_nand.c b/drivers/mtd/nand/mtk_nand.c
> index ddaa2ac..9c49121 100644
> --- a/drivers/mtd/nand/mtk_nand.c
> +++ b/drivers/mtd/nand/mtk_nand.c
> @@ -699,8 +699,8 @@ static int mtk_nfc_do_write_page(struct mtd_info
> *mtd, struct nand_chip *chip,
> }
>
> ret = readl_poll_timeout_atomic(nfc->regs + NFI_ADDRCNTR, reg,
> - (reg & CNTR_MASK) >=
> chip->ecc.steps,
> - 10, MTK_TIMEOUT);
> + ((reg & CNTR_MASK) >> 12) >=
> + chip->ecc.steps, 10,
Please keep '((reg & CNTR_MASK) >> 12) >= chip->ecc.steps' on the same
line.
If you want to comply with the 80 char lines, you can do:
ret = readl_poll_timeout_atomic(nfc->regs + NFI_ADDRCNTR, reg,
((reg & CNTR_MASK) >> 12) >= chip->ecc.steps,
10, MTK_TIMEOUT);
> if (ret)
> dev_err(dev, "hwecc write timeout\n");
>
> @@ -902,8 +902,8 @@ static int mtk_nfc_read_subpage(struct mtd_info
> *mtd, struct nand_chip *chip,
> dev_warn(nfc->dev, "read ahb/dma done timeout\n");
>
> rc = readl_poll_timeout_atomic(nfc->regs + NFI_BYTELEN, reg,
> - (reg & CNTR_MASK) >= sectors, 10,
> - MTK_TIMEOUT);
> + ((reg & CNTR_MASK) >> 12) >=
> sectors,
> + 10, MTK_TIMEOUT);
> if (rc < 0) {
> dev_err(nfc->dev, "subpage done timeout\n");
> bitflips = -EIO;
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox