linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* ubifs_recover_master_node: failed to recover master node
@ 2024-10-29  0:38 Chris Packham
  2024-10-29 21:13 ` Chris Packham
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2024-10-29  0:38 UTC (permalink / raw)
  To: linux-mtd@lists.infradead.org, Miquel Raynal,
	linux-spi@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org

(resend as plaintext)

Hi,

I recently added support for the SPI-NAND controller on the RTL9302C 
SoC[1]. I did most of the work against Linux 6.11 and it's working fine 
there. I recently rebased against the tip of Linus's tree (6.12-rc5) and 
found I was getting ubifs errors when mounting:

[    1.255191] spi-nand spi1.0: Macronix SPI NAND was found.
[    1.261283] spi-nand spi1.0: 256 MiB, block size: 128 KiB, page size: 
2048, OOB size: 64
[    1.271134] 2 fixed-partitions partitions found on MTD device spi1.0
[    1.278247] Creating 2 MTD partitions on "spi1.0":
[    1.283631] 0x000000000000-0x00000f000000 : "user"
[   20.481108] 0x00000f000000-0x000010000000 : "Reserved"
[   72.240347] ubi0: scanning is finished
[   72.270577] ubi0: attached mtd3 (name "user", size 240 MiB)
[   72.276815] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 
bytes
[   72.284537] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
[   72.292132] ubi0: VID header offset: 2048 (aligned 2048), data 
offset: 4096
[   72.299885] ubi0: good PEBs: 1920, bad PEBs: 0, corrupted PEBs: 0
[   72.306689] ubi0: user volume: 1, internal volumes: 1, max. volumes 
count: 128
[   72.314747] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, 
image sequence number: 252642230
[   72.324850] ubi0: available PEBs: 0, total reserved PEBs: 1920, PEBs 
reserved for bad PEB handling: 40
[   72.370123] ubi0: background thread "ubi_bgt0d" started, PID 141
[   72.470740] UBIFS (ubi0:0): Mounting in unauthenticated mode
[   72.490246] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, 
PID 144
[   72.528272] UBIFS error (ubi0:0 pid 143): ubifs_recover_master_node: 
failed to recover master node
[   72.550122] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
[   72.710720] UBIFS (ubi0:0): Mounting in unauthenticated mode
[   72.717447] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, 
PID 149
[   72.777602] UBIFS error (ubi0:0 pid 148): ubifs_recover_master_node: 
failed to recover master node
[   72.787792] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops

Full dmesg output is at[2]

git bisect lead me to commit 11813857864f ("mtd: spi-nand: macronix: 
Continuous read support"). Reverting the blamed commit from 6.12-rc5 
seems to avoid the problem. The flash chip on my board is a 
MX30LF2G28AD-TI. I'm not sure if there is a problem with 11813857864f or 
with my spi-mem driver that is exposed after support for continuous read 
is enabled.

Thanks,
Chris

--

[1] - 
https://lore.kernel.org/all/20241015225434.3970360-1-chris.packham@alliedtelesis.co.nz/
[2] - 
https://gist.github.com/cpackham-atlnz/66a0843362e8f8eb2c4f5c7ed01c5efe


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: ubifs_recover_master_node: failed to recover master node
  2024-10-29  0:38 ubifs_recover_master_node: failed to recover master node Chris Packham
@ 2024-10-29 21:13 ` Chris Packham
  2024-10-29 21:51   ` [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers Chris Packham
                     ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Chris Packham @ 2024-10-29 21:13 UTC (permalink / raw)
  To: linux-mtd@lists.infradead.org, Miquel Raynal,
	linux-spi@vger.kernel.org, broonie@kernel.org
  Cc: linux-kernel@vger.kernel.org


On 29/10/24 13:38, Chris Packham wrote:
> (resend as plaintext)
>
> Hi,
>
> I recently added support for the SPI-NAND controller on the RTL9302C 
> SoC[1]. I did most of the work against Linux 6.11 and it's working 
> fine there. I recently rebased against the tip of Linus's tree 
> (6.12-rc5) and found I was getting ubifs errors when mounting:
>
> [    1.255191] spi-nand spi1.0: Macronix SPI NAND was found.
> [    1.261283] spi-nand spi1.0: 256 MiB, block size: 128 KiB, page 
> size: 2048, OOB size: 64
> [    1.271134] 2 fixed-partitions partitions found on MTD device spi1.0
> [    1.278247] Creating 2 MTD partitions on "spi1.0":
> [    1.283631] 0x000000000000-0x00000f000000 : "user"
> [   20.481108] 0x00000f000000-0x000010000000 : "Reserved"
> [   72.240347] ubi0: scanning is finished
> [   72.270577] ubi0: attached mtd3 (name "user", size 240 MiB)
> [   72.276815] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 
> 126976 bytes
> [   72.284537] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page 
> size 2048
> [   72.292132] ubi0: VID header offset: 2048 (aligned 2048), data 
> offset: 4096
> [   72.299885] ubi0: good PEBs: 1920, bad PEBs: 0, corrupted PEBs: 0
> [   72.306689] ubi0: user volume: 1, internal volumes: 1, max. volumes 
> count: 128
> [   72.314747] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, 
> image sequence number: 252642230
> [   72.324850] ubi0: available PEBs: 0, total reserved PEBs: 1920, 
> PEBs reserved for bad PEB handling: 40
> [   72.370123] ubi0: background thread "ubi_bgt0d" started, PID 141
> [   72.470740] UBIFS (ubi0:0): Mounting in unauthenticated mode
> [   72.490246] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" 
> started, PID 144
> [   72.528272] UBIFS error (ubi0:0 pid 143): 
> ubifs_recover_master_node: failed to recover master node
> [   72.550122] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
> [   72.710720] UBIFS (ubi0:0): Mounting in unauthenticated mode
> [   72.717447] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" 
> started, PID 149
> [   72.777602] UBIFS error (ubi0:0 pid 148): 
> ubifs_recover_master_node: failed to recover master node
> [   72.787792] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
>
> Full dmesg output is at[2]
>
> git bisect lead me to commit 11813857864f ("mtd: spi-nand: macronix: 
> Continuous read support"). Reverting the blamed commit from 6.12-rc5 
> seems to avoid the problem. The flash chip on my board is a 
> MX30LF2G28AD-TI. I'm not sure if there is a problem with 11813857864f 
> or with my spi-mem driver that is exposed after support for continuous 
> read is enabled.
>
A bit of an update. The ubifs failure is from the is_empty() check in 
get_master_node(). It looks like portions of the LEB are 0 instead of 
0xff. I've also found if I avoid use the non-DMA path in my driver I 
don't have such a problem. I think there is at least one problem in my 
driver because I don't handle DMAing more than 0xffff bytes.


> Thanks,
> Chris
>
> -- 
>
> [1] - 
> https://lore.kernel.org/all/20241015225434.3970360-1-chris.packham@alliedtelesis.co.nz/
> [2] - 
> https://gist.github.com/cpackham-atlnz/66a0843362e8f8eb2c4f5c7ed01c5efe
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
  2024-10-29 21:13 ` Chris Packham
@ 2024-10-29 21:51   ` Chris Packham
  2024-10-30  6:42     ` kernel test robot
                       ` (2 more replies)
  2024-10-30 19:49   ` [PATCH v2] " Chris Packham
  2024-11-06 16:12   ` ubifs_recover_master_node: failed to recover master node Miquel Raynal
  2 siblings, 3 replies; 11+ messages in thread
From: Chris Packham @ 2024-10-29 21:51 UTC (permalink / raw)
  To: broonie, miquel.raynal; +Cc: linux-spi, linux-mtd, linux-kernel, Chris Packham

The RTL9300 has some limitations on the maximum DMA transfers possible.
For reads this is 2080 bytes (520*4) for writes this is 520 bytes. Deal
with this by splitting transfers into appropriately sized parts. Also
ensure that we are using the correct buffer when doing DMA writes.

Fixes: 42d20a6a61b8 ("spi: spi-mem: Add Realtek SPI-NAND controller")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/spi/spi-realtek-rtl-snand.c | 51 +++++++++++++++++++----------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-realtek-rtl-snand.c b/drivers/spi/spi-realtek-rtl-snand.c
index 23c42c8469e4..50b3240641e8 100644
--- a/drivers/spi/spi-realtek-rtl-snand.c
+++ b/drivers/spi/spi-realtek-rtl-snand.c
@@ -231,19 +231,25 @@ static int rtl_snand_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_
 
 static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
 {
+	unsigned int pos, nbytes;
 	int ret;
 	dma_addr_t buf_dma;
 	enum dma_data_direction dir;
-	u32 trig;
+	u32 trig, len, maxlen;
+	void *buf;
 
 	ret = rtl_snand_xfer_head(snand, cs, op);
 	if (ret)
 		goto out_deselect;
 
 	if (op->data.dir == SPI_MEM_DATA_IN) {
+		maxlen = 2080;
+		buf = op->data.buf.in;
 		dir = DMA_FROM_DEVICE;
 		trig = 0;
 	} else if (op->data.dir == SPI_MEM_DATA_OUT) {
+		maxlen = 520;
+		buf = op->data.buf.out;
 		dir = DMA_TO_DEVICE;
 		trig = 1;
 	} else {
@@ -251,7 +257,7 @@ static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_
 		goto out_deselect;
 	}
 
-	buf_dma = dma_map_single(snand->dev, op->data.buf.in, op->data.nbytes, dir);
+	buf_dma = dma_map_single(snand->dev, buf, op->data.nbytes, dir);
 	ret = dma_mapping_error(snand->dev, buf_dma);
 	if (ret)
 		goto out_deselect;
@@ -264,26 +270,37 @@ static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_
 	if (ret)
 		goto out_unmap;
 
-	reinit_completion(&snand->comp);
+	pos = 0;
+	len = op->data.nbytes;
 
-	ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma);
-	if (ret)
-		goto out_disable_int;
+	while (pos < len) {
+		nbytes = len - pos;
+		if (nbytes > maxlen)
+			nbytes = maxlen;
 
-	ret = regmap_write(snand->regmap, SNAFDLR,
-			   CMR_WID(op->data.buswidth) | (op->data.nbytes & 0xffff));
-	if (ret)
-		goto out_disable_int;
+		reinit_completion(&snand->comp);
 
-	ret = regmap_write(snand->regmap, SNAFDTR, trig);
-	if (ret)
-		goto out_disable_int;
+		ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma + pos);
+		if (ret)
+			goto out_disable_int;
 
-	if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
-		ret = -ETIMEDOUT;
+		pos += nbytes;
 
-	if (ret)
-		goto out_disable_int;
+		ret = regmap_write(snand->regmap, SNAFDLR,
+				CMR_WID(op->data.buswidth) | nbytes);
+		if (ret)
+			goto out_disable_int;
+
+		ret = regmap_write(snand->regmap, SNAFDTR, trig);
+		if (ret)
+			goto out_disable_int;
+
+		if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
+			ret = -ETIMEDOUT;
+
+		if (ret)
+			goto out_disable_int;
+	}
 
 out_disable_int:
 	regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, 0);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
  2024-10-29 21:51   ` [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers Chris Packham
@ 2024-10-30  6:42     ` kernel test robot
  2024-10-30 10:20     ` kernel test robot
  2024-10-30 19:53     ` kernel test robot
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2024-10-30  6:42 UTC (permalink / raw)
  To: Chris Packham, broonie, miquel.raynal
  Cc: oe-kbuild-all, linux-spi, linux-mtd, linux-kernel, Chris Packham

Hi Chris,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on next-20241029]
[cannot apply to linus/master v6.12-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Packham/spi-spi-mem-rtl-snand-Correctly-handle-DMA-transfers/20241030-055313
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20241029215159.1975844-1-chris.packham%40alliedtelesis.co.nz
patch subject: [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20241030/202410301452.7koB3V8S-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241030/202410301452.7koB3V8S-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410301452.7koB3V8S-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/spi/spi-realtek-rtl-snand.c: In function 'rtl_snand_dma_xfer':
>> drivers/spi/spi-realtek-rtl-snand.c:252:21: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     252 |                 buf = op->data.buf.out;
         |                     ^

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +/const +252 drivers/spi/spi-realtek-rtl-snand.c

   231	
   232	static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
   233	{
   234		unsigned int pos, nbytes;
   235		int ret;
   236		dma_addr_t buf_dma;
   237		enum dma_data_direction dir;
   238		u32 trig, len, maxlen;
   239		void *buf;
   240	
   241		ret = rtl_snand_xfer_head(snand, cs, op);
   242		if (ret)
   243			goto out_deselect;
   244	
   245		if (op->data.dir == SPI_MEM_DATA_IN) {
   246			maxlen = 2080;
   247			buf = op->data.buf.in;
   248			dir = DMA_FROM_DEVICE;
   249			trig = 0;
   250		} else if (op->data.dir == SPI_MEM_DATA_OUT) {
   251			maxlen = 520;
 > 252			buf = op->data.buf.out;
   253			dir = DMA_TO_DEVICE;
   254			trig = 1;
   255		} else {
   256			ret = -EOPNOTSUPP;
   257			goto out_deselect;
   258		}
   259	
   260		buf_dma = dma_map_single(snand->dev, buf, op->data.nbytes, dir);
   261		ret = dma_mapping_error(snand->dev, buf_dma);
   262		if (ret)
   263			goto out_deselect;
   264	
   265		ret = regmap_write(snand->regmap, SNAFDIR, SNAFDIR_DMA_IP);
   266		if (ret)
   267			goto out_unmap;
   268	
   269		ret = regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, SNAFCFR_DMA_IE);
   270		if (ret)
   271			goto out_unmap;
   272	
   273		pos = 0;
   274		len = op->data.nbytes;
   275	
   276		while (pos < len) {
   277			nbytes = len - pos;
   278			if (nbytes > maxlen)
   279				nbytes = maxlen;
   280	
   281			reinit_completion(&snand->comp);
   282	
   283			ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma + pos);
   284			if (ret)
   285				goto out_disable_int;
   286	
   287			pos += nbytes;
   288	
   289			ret = regmap_write(snand->regmap, SNAFDLR,
   290					CMR_WID(op->data.buswidth) | nbytes);
   291			if (ret)
   292				goto out_disable_int;
   293	
   294			ret = regmap_write(snand->regmap, SNAFDTR, trig);
   295			if (ret)
   296				goto out_disable_int;
   297	
   298			if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
   299				ret = -ETIMEDOUT;
   300	
   301			if (ret)
   302				goto out_disable_int;
   303		}
   304	
   305	out_disable_int:
   306		regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, 0);
   307	out_unmap:
   308		dma_unmap_single(snand->dev, buf_dma, op->data.nbytes, dir);
   309	out_deselect:
   310		rtl_snand_xfer_tail(snand, cs);
   311	
   312		if (ret)
   313			dev_err(snand->dev, "transfer failed %d\n", ret);
   314	
   315		return ret;
   316	}
   317	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
  2024-10-29 21:51   ` [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers Chris Packham
  2024-10-30  6:42     ` kernel test robot
@ 2024-10-30 10:20     ` kernel test robot
  2024-10-30 19:53     ` kernel test robot
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2024-10-30 10:20 UTC (permalink / raw)
  To: Chris Packham, broonie, miquel.raynal
  Cc: llvm, oe-kbuild-all, linux-spi, linux-mtd, linux-kernel,
	Chris Packham

Hi Chris,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-spi/for-next]
[also build test ERROR on next-20241029]
[cannot apply to linus/master v6.12-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Packham/spi-spi-mem-rtl-snand-Correctly-handle-DMA-transfers/20241030-055313
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20241029215159.1975844-1-chris.packham%40alliedtelesis.co.nz
patch subject: [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241030/202410301731.MOPjsQ0R-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241030/202410301731.MOPjsQ0R-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410301731.MOPjsQ0R-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/spi/spi-realtek-rtl-snand.c:4:
   In file included from include/linux/dma-mapping.h:8:
   In file included from include/linux/device.h:32:
   In file included from include/linux/device/driver.h:21:
   In file included from include/linux/module.h:19:
   In file included from include/linux/elf.h:6:
   In file included from arch/s390/include/asm/elf.h:181:
   In file included from arch/s390/include/asm/mmu_context.h:11:
   In file included from arch/s390/include/asm/pgalloc.h:18:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/spi/spi-realtek-rtl-snand.c:4:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from drivers/spi/spi-realtek-rtl-snand.c:4:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from drivers/spi/spi-realtek-rtl-snand.c:4:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> drivers/spi/spi-realtek-rtl-snand.c:252:7: error: assigning to 'void *' from 'const void *const' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
     252 |                 buf = op->data.buf.out;
         |                     ^ ~~~~~~~~~~~~~~~~
   16 warnings and 1 error generated.


vim +252 drivers/spi/spi-realtek-rtl-snand.c

   231	
   232	static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
   233	{
   234		unsigned int pos, nbytes;
   235		int ret;
   236		dma_addr_t buf_dma;
   237		enum dma_data_direction dir;
   238		u32 trig, len, maxlen;
   239		void *buf;
   240	
   241		ret = rtl_snand_xfer_head(snand, cs, op);
   242		if (ret)
   243			goto out_deselect;
   244	
   245		if (op->data.dir == SPI_MEM_DATA_IN) {
   246			maxlen = 2080;
   247			buf = op->data.buf.in;
   248			dir = DMA_FROM_DEVICE;
   249			trig = 0;
   250		} else if (op->data.dir == SPI_MEM_DATA_OUT) {
   251			maxlen = 520;
 > 252			buf = op->data.buf.out;
   253			dir = DMA_TO_DEVICE;
   254			trig = 1;
   255		} else {
   256			ret = -EOPNOTSUPP;
   257			goto out_deselect;
   258		}
   259	
   260		buf_dma = dma_map_single(snand->dev, buf, op->data.nbytes, dir);
   261		ret = dma_mapping_error(snand->dev, buf_dma);
   262		if (ret)
   263			goto out_deselect;
   264	
   265		ret = regmap_write(snand->regmap, SNAFDIR, SNAFDIR_DMA_IP);
   266		if (ret)
   267			goto out_unmap;
   268	
   269		ret = regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, SNAFCFR_DMA_IE);
   270		if (ret)
   271			goto out_unmap;
   272	
   273		pos = 0;
   274		len = op->data.nbytes;
   275	
   276		while (pos < len) {
   277			nbytes = len - pos;
   278			if (nbytes > maxlen)
   279				nbytes = maxlen;
   280	
   281			reinit_completion(&snand->comp);
   282	
   283			ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma + pos);
   284			if (ret)
   285				goto out_disable_int;
   286	
   287			pos += nbytes;
   288	
   289			ret = regmap_write(snand->regmap, SNAFDLR,
   290					CMR_WID(op->data.buswidth) | nbytes);
   291			if (ret)
   292				goto out_disable_int;
   293	
   294			ret = regmap_write(snand->regmap, SNAFDTR, trig);
   295			if (ret)
   296				goto out_disable_int;
   297	
   298			if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
   299				ret = -ETIMEDOUT;
   300	
   301			if (ret)
   302				goto out_disable_int;
   303		}
   304	
   305	out_disable_int:
   306		regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, 0);
   307	out_unmap:
   308		dma_unmap_single(snand->dev, buf_dma, op->data.nbytes, dir);
   309	out_deselect:
   310		rtl_snand_xfer_tail(snand, cs);
   311	
   312		if (ret)
   313			dev_err(snand->dev, "transfer failed %d\n", ret);
   314	
   315		return ret;
   316	}
   317	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
  2024-10-29 21:13 ` Chris Packham
  2024-10-29 21:51   ` [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers Chris Packham
@ 2024-10-30 19:49   ` Chris Packham
  2024-11-04 14:06     ` Mark Brown
  2024-11-06 16:12   ` ubifs_recover_master_node: failed to recover master node Miquel Raynal
  2 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2024-10-30 19:49 UTC (permalink / raw)
  To: broonie, miquel.raynal; +Cc: linux-spi, linux-mtd, linux-kernel, Chris Packham

The RTL9300 has some limitations on the maximum DMA transfers possible.
For reads this is 2080 bytes (520*4) for writes this is 520 bytes. Deal
with this by splitting transfers into appropriately sized parts.

Fixes: 42d20a6a61b8 ("spi: spi-mem: Add Realtek SPI-NAND controller")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v2:
    - Drop unecessary "also" change. data.in and data.out point to the same
      memory but the latter is marked as const which causes some compiler
      warnings when we're trying to get a dma mapping for it.

 drivers/spi/spi-realtek-rtl-snand.c | 46 +++++++++++++++++++----------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-realtek-rtl-snand.c b/drivers/spi/spi-realtek-rtl-snand.c
index 23c42c8469e4..cd0484041147 100644
--- a/drivers/spi/spi-realtek-rtl-snand.c
+++ b/drivers/spi/spi-realtek-rtl-snand.c
@@ -231,19 +231,22 @@ static int rtl_snand_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_
 
 static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
 {
+	unsigned int pos, nbytes;
 	int ret;
 	dma_addr_t buf_dma;
 	enum dma_data_direction dir;
-	u32 trig;
+	u32 trig, len, maxlen;
 
 	ret = rtl_snand_xfer_head(snand, cs, op);
 	if (ret)
 		goto out_deselect;
 
 	if (op->data.dir == SPI_MEM_DATA_IN) {
+		maxlen = 2080;
 		dir = DMA_FROM_DEVICE;
 		trig = 0;
 	} else if (op->data.dir == SPI_MEM_DATA_OUT) {
+		maxlen = 520;
 		dir = DMA_TO_DEVICE;
 		trig = 1;
 	} else {
@@ -264,26 +267,37 @@ static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_
 	if (ret)
 		goto out_unmap;
 
-	reinit_completion(&snand->comp);
+	pos = 0;
+	len = op->data.nbytes;
 
-	ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma);
-	if (ret)
-		goto out_disable_int;
+	while (pos < len) {
+		nbytes = len - pos;
+		if (nbytes > maxlen)
+			nbytes = maxlen;
 
-	ret = regmap_write(snand->regmap, SNAFDLR,
-			   CMR_WID(op->data.buswidth) | (op->data.nbytes & 0xffff));
-	if (ret)
-		goto out_disable_int;
+		reinit_completion(&snand->comp);
 
-	ret = regmap_write(snand->regmap, SNAFDTR, trig);
-	if (ret)
-		goto out_disable_int;
+		ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma + pos);
+		if (ret)
+			goto out_disable_int;
 
-	if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
-		ret = -ETIMEDOUT;
+		pos += nbytes;
 
-	if (ret)
-		goto out_disable_int;
+		ret = regmap_write(snand->regmap, SNAFDLR,
+				CMR_WID(op->data.buswidth) | nbytes);
+		if (ret)
+			goto out_disable_int;
+
+		ret = regmap_write(snand->regmap, SNAFDTR, trig);
+		if (ret)
+			goto out_disable_int;
+
+		if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
+			ret = -ETIMEDOUT;
+
+		if (ret)
+			goto out_disable_int;
+	}
 
 out_disable_int:
 	regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, 0);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
  2024-10-29 21:51   ` [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers Chris Packham
  2024-10-30  6:42     ` kernel test robot
  2024-10-30 10:20     ` kernel test robot
@ 2024-10-30 19:53     ` kernel test robot
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2024-10-30 19:53 UTC (permalink / raw)
  To: Chris Packham, broonie, miquel.raynal
  Cc: oe-kbuild-all, linux-spi, linux-mtd, linux-kernel, Chris Packham

Hi Chris,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on next-20241030]
[cannot apply to linus/master v6.12-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Packham/spi-spi-mem-rtl-snand-Correctly-handle-DMA-transfers/20241030-055313
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20241029215159.1975844-1-chris.packham%40alliedtelesis.co.nz
patch subject: [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
config: nios2-randconfig-r131-20241030 (https://download.01.org/0day-ci/archive/20241031/202410310358.GyKQwhxO-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20241031/202410310358.GyKQwhxO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410310358.GyKQwhxO-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/spi/spi-realtek-rtl-snand.c:252:21: sparse: sparse: incorrect type in assignment (different modifiers) @@     expected void *[assigned] buf @@     got void const *const out @@
   drivers/spi/spi-realtek-rtl-snand.c:252:21: sparse:     expected void *[assigned] buf
   drivers/spi/spi-realtek-rtl-snand.c:252:21: sparse:     got void const *const out

vim +252 drivers/spi/spi-realtek-rtl-snand.c

   231	
   232	static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
   233	{
   234		unsigned int pos, nbytes;
   235		int ret;
   236		dma_addr_t buf_dma;
   237		enum dma_data_direction dir;
   238		u32 trig, len, maxlen;
   239		void *buf;
   240	
   241		ret = rtl_snand_xfer_head(snand, cs, op);
   242		if (ret)
   243			goto out_deselect;
   244	
   245		if (op->data.dir == SPI_MEM_DATA_IN) {
   246			maxlen = 2080;
   247			buf = op->data.buf.in;
   248			dir = DMA_FROM_DEVICE;
   249			trig = 0;
   250		} else if (op->data.dir == SPI_MEM_DATA_OUT) {
   251			maxlen = 520;
 > 252			buf = op->data.buf.out;
   253			dir = DMA_TO_DEVICE;
   254			trig = 1;
   255		} else {
   256			ret = -EOPNOTSUPP;
   257			goto out_deselect;
   258		}
   259	
   260		buf_dma = dma_map_single(snand->dev, buf, op->data.nbytes, dir);
   261		ret = dma_mapping_error(snand->dev, buf_dma);
   262		if (ret)
   263			goto out_deselect;
   264	
   265		ret = regmap_write(snand->regmap, SNAFDIR, SNAFDIR_DMA_IP);
   266		if (ret)
   267			goto out_unmap;
   268	
   269		ret = regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, SNAFCFR_DMA_IE);
   270		if (ret)
   271			goto out_unmap;
   272	
   273		pos = 0;
   274		len = op->data.nbytes;
   275	
   276		while (pos < len) {
   277			nbytes = len - pos;
   278			if (nbytes > maxlen)
   279				nbytes = maxlen;
   280	
   281			reinit_completion(&snand->comp);
   282	
   283			ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma + pos);
   284			if (ret)
   285				goto out_disable_int;
   286	
   287			pos += nbytes;
   288	
   289			ret = regmap_write(snand->regmap, SNAFDLR,
   290					CMR_WID(op->data.buswidth) | nbytes);
   291			if (ret)
   292				goto out_disable_int;
   293	
   294			ret = regmap_write(snand->regmap, SNAFDTR, trig);
   295			if (ret)
   296				goto out_disable_int;
   297	
   298			if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
   299				ret = -ETIMEDOUT;
   300	
   301			if (ret)
   302				goto out_disable_int;
   303		}
   304	
   305	out_disable_int:
   306		regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, 0);
   307	out_unmap:
   308		dma_unmap_single(snand->dev, buf_dma, op->data.nbytes, dir);
   309	out_deselect:
   310		rtl_snand_xfer_tail(snand, cs);
   311	
   312		if (ret)
   313			dev_err(snand->dev, "transfer failed %d\n", ret);
   314	
   315		return ret;
   316	}
   317	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
  2024-10-30 19:49   ` [PATCH v2] " Chris Packham
@ 2024-11-04 14:06     ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2024-11-04 14:06 UTC (permalink / raw)
  To: miquel.raynal, Chris Packham; +Cc: linux-spi, linux-mtd, linux-kernel

On Thu, 31 Oct 2024 08:49:20 +1300, Chris Packham wrote:
> The RTL9300 has some limitations on the maximum DMA transfers possible.
> For reads this is 2080 bytes (520*4) for writes this is 520 bytes. Deal
> with this by splitting transfers into appropriately sized parts.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: spi-mem: rtl-snand: Correctly handle DMA transfers
      commit: 25d284715845a465a1a3693a09cf8b6ab8bd9caf

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: ubifs_recover_master_node: failed to recover master node
       [not found] <7eaf332e-9439-4d4c-a2ea-d963e41f44f2@alliedtelesis.co.nz>
@ 2024-11-06 15:35 ` Miquel Raynal
  2024-11-06 19:38   ` Chris Packham
  0 siblings, 1 reply; 11+ messages in thread
From: Miquel Raynal @ 2024-11-06 15:35 UTC (permalink / raw)
  To: Chris Packham
  Cc: linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org,
	linux-kernel@vger.kernel.org


Hi Chris,

On 29/10/2024 at 13:37:31 +13, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> Hi,
>
> I recently added support for the SPI-NAND controller on the RTL9302C SoC[1]. I did most of the work against Linux 6.11
> and it's working fine there. I recently rebased against the tip of Linus's tree (6.12-rc5) and found I was getting ubifs
> errors when mounting:
>
> [    1.255191] spi-nand spi1.0: Macronix SPI NAND was found.
> [    1.261283] spi-nand spi1.0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 64
> [    1.271134] 2 fixed-partitions partitions found on MTD device spi1.0
> [    1.278247] Creating 2 MTD partitions on "spi1.0":
> [    1.283631] 0x000000000000-0x00000f000000 : "user"
> [   20.481108] 0x00000f000000-0x000010000000 : "Reserved"
> [   72.240347] ubi0: scanning is finished
> [   72.270577] ubi0: attached mtd3 (name "user", size 240 MiB)
> [   72.276815] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
> [   72.284537] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
> [   72.292132] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
> [   72.299885] ubi0: good PEBs: 1920, bad PEBs: 0, corrupted PEBs: 0
> [   72.306689] ubi0: user volume: 1, internal volumes: 1, max. volumes count: 128
> [   72.314747] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 252642230
> [   72.324850] ubi0: available PEBs: 0, total reserved PEBs: 1920, PEBs reserved for bad PEB handling: 40
> [   72.370123] ubi0: background thread "ubi_bgt0d" started, PID 141
> [   72.470740] UBIFS (ubi0:0): Mounting in unauthenticated mode
> [   72.490246] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, PID 144
> [   72.528272] UBIFS error (ubi0:0 pid 143): ubifs_recover_master_node: failed to recover master node
> [   72.550122] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
> [   72.710720] UBIFS (ubi0:0): Mounting in unauthenticated mode
> [   72.717447] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, PID 149
> [   72.777602] UBIFS error (ubi0:0 pid 148): ubifs_recover_master_node: failed to recover master node
> [   72.787792] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
>
> Full dmesg output is at[2]
>
> git bisect lead me to commit 11813857864f ("mtd: spi-nand: macronix: Continuous read support"). Reverting the blamed
> commit from 6.12-rc5 seems to avoid the problem. The flash chip on my board is a MX30LF2G28AD-TI. I'm not sure if there
> is a problem with 11813857864f or with my spi-mem driver that is
> exposed after support for continuous read is enabled.

Crap. I had a look, and TBH I don't know. The only thing I see in your
driver might be the DMA vs PIO choice. Could you try to always return
false from rtl_snand_dma_op()?

However you say you're using an MX30* device, this is a raw NAND chip,
SPI-NAND chips are I believe starting with MX35* in their IDs, no?

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: ubifs_recover_master_node: failed to recover master node
  2024-10-29 21:13 ` Chris Packham
  2024-10-29 21:51   ` [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers Chris Packham
  2024-10-30 19:49   ` [PATCH v2] " Chris Packham
@ 2024-11-06 16:12   ` Miquel Raynal
  2 siblings, 0 replies; 11+ messages in thread
From: Miquel Raynal @ 2024-11-06 16:12 UTC (permalink / raw)
  To: Chris Packham
  Cc: linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org,
	broonie@kernel.org, linux-kernel@vger.kernel.org


Hi Chris,

On 30/10/2024 at 10:13:45 +13, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> On 29/10/24 13:38, Chris Packham wrote:
>> (resend as plaintext)
>>
>> Hi,
>>
>> I recently added support for the SPI-NAND controller on the RTL9302C
>> SoC[1]. I did most of the work against Linux 6.11 and it's working
>> fine there. I recently rebased against the tip of Linus's tree
>> (6.12-rc5) and found I was getting ubifs errors when mounting:
>>
>> [    1.255191] spi-nand spi1.0: Macronix SPI NAND was found.
>> [    1.261283] spi-nand spi1.0: 256 MiB, block size: 128 KiB, page
>> size: 2048, OOB size: 64
>> [    1.271134] 2 fixed-partitions partitions found on MTD device spi1.0
>> [    1.278247] Creating 2 MTD partitions on "spi1.0":
>> [    1.283631] 0x000000000000-0x00000f000000 : "user"
>> [   20.481108] 0x00000f000000-0x000010000000 : "Reserved"
>> [   72.240347] ubi0: scanning is finished
>> [   72.270577] ubi0: attached mtd3 (name "user", size 240 MiB)
>> [   72.276815] ubi0: PEB size: 131072 bytes (128 KiB), LEB size:
>> 126976 bytes
>> [   72.284537] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page
>> size 2048
>> [   72.292132] ubi0: VID header offset: 2048 (aligned 2048), data
>> offset: 4096
>> [   72.299885] ubi0: good PEBs: 1920, bad PEBs: 0, corrupted PEBs: 0
>> [   72.306689] ubi0: user volume: 1, internal volumes: 1, max. volumes
>> count: 128
>> [   72.314747] ubi0: max/mean erase counter: 1/0, WL threshold: 4096,
>> image sequence number: 252642230
>> [   72.324850] ubi0: available PEBs: 0, total reserved PEBs: 1920,
>> PEBs reserved for bad PEB handling: 40
>> [   72.370123] ubi0: background thread "ubi_bgt0d" started, PID 141
>> [   72.470740] UBIFS (ubi0:0): Mounting in unauthenticated mode
>> [   72.490246] UBIFS (ubi0:0): background thread "ubifs_bgt0_0"
>> started, PID 144
>> [   72.528272] UBIFS error (ubi0:0 pid 143):
>> ubifs_recover_master_node: failed to recover master node
>> [   72.550122] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
>> [   72.710720] UBIFS (ubi0:0): Mounting in unauthenticated mode
>> [   72.717447] UBIFS (ubi0:0): background thread "ubifs_bgt0_0"
>> started, PID 149
>> [   72.777602] UBIFS error (ubi0:0 pid 148):
>> ubifs_recover_master_node: failed to recover master node
>> [   72.787792] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
>>
>> Full dmesg output is at[2]
>>
>> git bisect lead me to commit 11813857864f ("mtd: spi-nand: macronix:
>> Continuous read support"). Reverting the blamed commit from 6.12-rc5
>> seems to avoid the problem. The flash chip on my board is a
>> MX30LF2G28AD-TI. I'm not sure if there is a problem with 11813857864f
>> or with my spi-mem driver that is exposed after support for continuous
>> read is enabled.
>>
> A bit of an update. The ubifs failure is from the is_empty() check in
> get_master_node(). It looks like portions of the LEB are 0 instead of
> 0xff. I've also found if I avoid use the non-DMA path in my driver I
> don't have such a problem. I think there is at least one problem in my
> driver because I don't handle DMAing more than 0xffff bytes.

I am going through my mails in a chronological order :-)

Glad to see you found a lead. I was already a bit suspicious about the
DMA path, glad to see we might narrow down the problem.

Is the 0xffff limitation a hard constraint or is it just a pure software
constraint? If we reach a hard constraint, maybe you should check that
when you decide which path you take.

Miquèl

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: ubifs_recover_master_node: failed to recover master node
  2024-11-06 15:35 ` Miquel Raynal
@ 2024-11-06 19:38   ` Chris Packham
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Packham @ 2024-11-06 19:38 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org,
	linux-kernel@vger.kernel.org

Hi Miquel,

On 7/11/24 04:35, Miquel Raynal wrote:
> Hi Chris,
>
> On 29/10/2024 at 13:37:31 +13, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
>
>> Hi,
>>
>> I recently added support for the SPI-NAND controller on the RTL9302C SoC[1]. I did most of the work against Linux 6.11
>> and it's working fine there. I recently rebased against the tip of Linus's tree (6.12-rc5) and found I was getting ubifs
>> errors when mounting:
>>
>> [    1.255191] spi-nand spi1.0: Macronix SPI NAND was found.
>> [    1.261283] spi-nand spi1.0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 64
>> [    1.271134] 2 fixed-partitions partitions found on MTD device spi1.0
>> [    1.278247] Creating 2 MTD partitions on "spi1.0":
>> [    1.283631] 0x000000000000-0x00000f000000 : "user"
>> [   20.481108] 0x00000f000000-0x000010000000 : "Reserved"
>> [   72.240347] ubi0: scanning is finished
>> [   72.270577] ubi0: attached mtd3 (name "user", size 240 MiB)
>> [   72.276815] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
>> [   72.284537] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
>> [   72.292132] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
>> [   72.299885] ubi0: good PEBs: 1920, bad PEBs: 0, corrupted PEBs: 0
>> [   72.306689] ubi0: user volume: 1, internal volumes: 1, max. volumes count: 128
>> [   72.314747] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 252642230
>> [   72.324850] ubi0: available PEBs: 0, total reserved PEBs: 1920, PEBs reserved for bad PEB handling: 40
>> [   72.370123] ubi0: background thread "ubi_bgt0d" started, PID 141
>> [   72.470740] UBIFS (ubi0:0): Mounting in unauthenticated mode
>> [   72.490246] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, PID 144
>> [   72.528272] UBIFS error (ubi0:0 pid 143): ubifs_recover_master_node: failed to recover master node
>> [   72.550122] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
>> [   72.710720] UBIFS (ubi0:0): Mounting in unauthenticated mode
>> [   72.717447] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, PID 149
>> [   72.777602] UBIFS error (ubi0:0 pid 148): ubifs_recover_master_node: failed to recover master node
>> [   72.787792] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" stops
>>
>> Full dmesg output is at[2]
>>
>> git bisect lead me to commit 11813857864f ("mtd: spi-nand: macronix: Continuous read support"). Reverting the blamed
>> commit from 6.12-rc5 seems to avoid the problem. The flash chip on my board is a MX30LF2G28AD-TI. I'm not sure if there
>> is a problem with 11813857864f or with my spi-mem driver that is
>> exposed after support for continuous read is enabled.
> Crap. I had a look, and TBH I don't know. The only thing I see in your
> driver might be the DMA vs PIO choice. Could you try to always return
> false from rtl_snand_dma_op()?

It turned out the limitation was in my DMA support. With the fix for 
that[1] your changes are fine. I'm a little surprised I never hit 
problems with DMA prior to the continuous read changes but I guess the 
page reads would have been under the limit and my testing probably 
didn't trigger a big enough write.

> However you say you're using an MX30* device, this is a raw NAND chip,
> SPI-NAND chips are I believe starting with MX35* in their IDs, no?
I think I copied that part number off the wrong datasheet in my unsorted 
Downloads directory. The schematic for the board I have says 
MX35LF2GE4AD-Z4 and the correct datasheet has all the right things about 
SPI-NAND and continuous read.
>
> Thanks,
> Miquèl


[1] - 
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?id=25d284715845a465a1a3693a09cf8b6ab8bd9caf

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-11-06 19:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-29  0:38 ubifs_recover_master_node: failed to recover master node Chris Packham
2024-10-29 21:13 ` Chris Packham
2024-10-29 21:51   ` [PATCH] spi: spi-mem: rtl-snand: Correctly handle DMA transfers Chris Packham
2024-10-30  6:42     ` kernel test robot
2024-10-30 10:20     ` kernel test robot
2024-10-30 19:53     ` kernel test robot
2024-10-30 19:49   ` [PATCH v2] " Chris Packham
2024-11-04 14:06     ` Mark Brown
2024-11-06 16:12   ` ubifs_recover_master_node: failed to recover master node Miquel Raynal
     [not found] <7eaf332e-9439-4d4c-a2ea-d963e41f44f2@alliedtelesis.co.nz>
2024-11-06 15:35 ` Miquel Raynal
2024-11-06 19:38   ` Chris Packham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).