Re: doing lots of disk writes causes oom killer to kill processes
On 12 March 2013 03:15, Hillf Danton dhi...@gmail.com wrote: On 11 March 2013 13:15, Michal Suchanek hramr...@gmail.com wrote: On 8 February 2013 17:31, Michal Suchanek hramr...@gmail.com wrote: Hello, I am dealing with VM disk images and performing something like wiping free space to prepare image for compressing and storing on server or copying it to external USB disk causes 1) system lockup in order of a few tens of seconds when all CPU cores are 100% used by system and the machine is basicaly unusable 2) oom killer killing processes This all on system with 8G ram so there should be plenty space to work with. This happens with kernels 3.6.4 or 3.7.1 With earlier kernel versions (some 3.0 or 3.2 kernels) this was not a problem even with less ram. I have vm.swappiness = 0 set for a long time already. I did some testing with 3.7.1 and with swappiness as much as 75 the kernel still causes all cores to loop somewhere in system when writing lots of data to disk. With swappiness as much as 90 processes still get killed on large disk writes. Given that the max is 100 the interval in which mm works at all is going to be very narrow, less than 10% of the paramater range. This is a severe regression as is the cpu time consumed by the kernel. The io scheduler is the default cfq. If you have any idea what to try other than downgrading to an earlier unaffected kernel I would like to hear. Can you try commit 3cf23841b4b7(mm/vmscan.c: avoid possible deadlock caused by too_many_isolated())? Or try 3.8 and/or 3.9, additionally? Hello, in the meantime I tried setting io scheduler to deadline because I remember using that one in my self-built kernels due to cfq breaking some obscure block driver. With the deadline io scheduler I can set swappiness back to 0 and the system works normally even for moderate amount of IO - restoring disk images from network. This would cause lockups and oom killer running loose with the cfq scheduler. So I guess I found what breaks the system and it is not so much the kernel version. It's using pre-built kernels with the default scheduler. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
On 12 March 2013 03:15, Hillf Danton dhi...@gmail.com wrote: On 11 March 2013 13:15, Michal Suchanek hramr...@gmail.com wrote: On 8 February 2013 17:31, Michal Suchanek hramr...@gmail.com wrote: Hello, I am dealing with VM disk images and performing something like wiping free space to prepare image for compressing and storing on server or copying it to external USB disk causes 1) system lockup in order of a few tens of seconds when all CPU cores are 100% used by system and the machine is basicaly unusable 2) oom killer killing processes This all on system with 8G ram so there should be plenty space to work with. This happens with kernels 3.6.4 or 3.7.1 With earlier kernel versions (some 3.0 or 3.2 kernels) this was not a problem even with less ram. I have vm.swappiness = 0 set for a long time already. I did some testing with 3.7.1 and with swappiness as much as 75 the kernel still causes all cores to loop somewhere in system when writing lots of data to disk. With swappiness as much as 90 processes still get killed on large disk writes. Given that the max is 100 the interval in which mm works at all is going to be very narrow, less than 10% of the paramater range. This is a severe regression as is the cpu time consumed by the kernel. The io scheduler is the default cfq. If you have any idea what to try other than downgrading to an earlier unaffected kernel I would like to hear. Can you try commit 3cf23841b4b7(mm/vmscan.c: avoid possible deadlock caused by too_many_isolated())? Or try 3.8 and/or 3.9, additionally? Hello, with deadline IO scheduler I experience this issue less often but it still happens. I am on 3.9.6 Debian kernel so 3.8 did not fix this problem. Do you have some idea what to log so that useful information about the lockup is gathered? Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
On 5 September 2013 12:12, Michal Suchanek hramr...@gmail.com wrote:
Hello
On 26 August 2013 15:51, Michal Suchanek hramr...@gmail.com wrote:
On 12 March 2013 03:15, Hillf Danton dhi...@gmail.com wrote:
On 11 March 2013 13:15, Michal Suchanek hramr...@gmail.com wrote:
On 8 February 2013 17:31, Michal Suchanek hramr...@gmail.com wrote:
Hello,
I am dealing with VM disk images and performing something like wiping
free space to prepare image for compressing and storing on server or
copying it to external USB disk causes
1) system lockup in order of a few tens of seconds when all CPU cores
are 100% used by system and the machine is basicaly unusable
2) oom killer killing processes
This all on system with 8G ram so there should be plenty space to work
with.
This happens with kernels 3.6.4 or 3.7.1
With earlier kernel versions (some 3.0 or 3.2 kernels) this was not a
problem even with less ram.
I have vm.swappiness = 0 set for a long time already.
I did some testing with 3.7.1 and with swappiness as much as 75 the
kernel still causes all cores to loop somewhere in system when writing
lots of data to disk.
With swappiness as much as 90 processes still get killed on large disk
writes.
Given that the max is 100 the interval in which mm works at all is
going to be very narrow, less than 10% of the paramater range. This is
a severe regression as is the cpu time consumed by the kernel.
The io scheduler is the default cfq.
If you have any idea what to try other than downgrading to an earlier
unaffected kernel I would like to hear.
Can you try commit 3cf23841b4b7(mm/vmscan.c: avoid possible
deadlock caused by too_many_isolated())?
Or try 3.8 and/or 3.9, additionally?
Hello,
with deadline IO scheduler I experience this issue less often but it
still happens.
I am on 3.9.6 Debian kernel so 3.8 did not fix this problem.
Do you have some idea what to log so that useful information about the
lockup is gathered?
This appears to be fixed in vanilla 3.11 kernel.
I still get short intermittent lockups and cpu usage spikes up to 20%
on a core but nowhere near the minute+ long lockups with all cores
100% on earlier kernels.
So I did more testing on the 3.11 kernel and while it works OK with
tar you can get severe lockups with mc or kvm. The difference is
probably the fact that sane tools do fsync() on files they close
forcing the file to write out and the kernel returning possible write
errors before they move on to next file.
With kvm writing to a file used as virtual disk the system would stall
indefinitely until the disk driver in the emulated system would time
out, return disk IO error, and the emulated system would stop writing.
In top I see all CPU cores 90%+ in wait. System is unusable. With mc
the lockups would be indefinite, probably because there is no timeout
on writing a file in mc.
I tried tuning swappiness and eleveators but the the basic problem is
solved by neither: the dirty buffers fill up memory and system stalls
trying to resolve the situation.
Obviously the kernel puts off writing any dirty buffers until the
memory pressure is overwhelming and the vmm flops.
At least the OOM killer does not get invoked anymore since there is
lots of memory - just Linux does not know how to use it.
The solution to this problem is quite simple - use the ancient
userspace bdflushd or what it was called. I emulate it with
{ while true ; do sleep 5; sync ; done }
The system performance suddenly increases - to the awesome Debian stable levels.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
On 17 September 2013 23:13, Jan Kara j...@suse.cz wrote: Hello, On Tue 17-09-13 15:31:31, Michal Suchanek wrote: On 5 September 2013 12:12, Michal Suchanek hramr...@gmail.com wrote: On 26 August 2013 15:51, Michal Suchanek hramr...@gmail.com wrote: On 12 March 2013 03:15, Hillf Danton dhi...@gmail.com wrote: On 11 March 2013 13:15, Michal Suchanek hramr...@gmail.com wrote: On 8 February 2013 17:31, Michal Suchanek hramr...@gmail.com wrote: Hello, I am dealing with VM disk images and performing something like wiping free space to prepare image for compressing and storing on server or copying it to external USB disk causes 1) system lockup in order of a few tens of seconds when all CPU cores are 100% used by system and the machine is basicaly unusable 2) oom killer killing processes This all on system with 8G ram so there should be plenty space to work with. This happens with kernels 3.6.4 or 3.7.1 With earlier kernel versions (some 3.0 or 3.2 kernels) this was not a problem even with less ram. I have vm.swappiness = 0 set for a long time already. I did some testing with 3.7.1 and with swappiness as much as 75 the kernel still causes all cores to loop somewhere in system when writing lots of data to disk. With swappiness as much as 90 processes still get killed on large disk writes. Given that the max is 100 the interval in which mm works at all is going to be very narrow, less than 10% of the paramater range. This is a severe regression as is the cpu time consumed by the kernel. The io scheduler is the default cfq. If you have any idea what to try other than downgrading to an earlier unaffected kernel I would like to hear. Can you try commit 3cf23841b4b7(mm/vmscan.c: avoid possible deadlock caused by too_many_isolated())? Or try 3.8 and/or 3.9, additionally? Hello, with deadline IO scheduler I experience this issue less often but it still happens. I am on 3.9.6 Debian kernel so 3.8 did not fix this problem. Do you have some idea what to log so that useful information about the lockup is gathered? This appears to be fixed in vanilla 3.11 kernel. I still get short intermittent lockups and cpu usage spikes up to 20% on a core but nowhere near the minute+ long lockups with all cores 100% on earlier kernels. So I did more testing on the 3.11 kernel and while it works OK with tar you can get severe lockups with mc or kvm. The difference is probably the fact that sane tools do fsync() on files they close forcing the file to write out and the kernel returning possible write errors before they move on to next file. Sorry for chiming in a bit late. But is this really writing to a normal disk? SATA drive or something else? It's a LVM volume on a SATA drive. I sometimes use USB disks as well but most of the time it's SATA or eSATA. With kvm writing to a file used as virtual disk the system would stall indefinitely until the disk driver in the emulated system would time out, return disk IO error, and the emulated system would stop writing. In top I see all CPU cores 90%+ in wait. System is unusable. With mc the lockups would be indefinite, probably because there is no timeout on writing a file in mc. I tried tuning swappiness and eleveators but the the basic problem is solved by neither: the dirty buffers fill up memory and system stalls trying to resolve the situation. This is really strange. There is /proc/sys/vm/dirty_ratio, which limits amount of dirty memory. By default it is set to 20% of memory which tends to be too much for 8 GB machine. Can you set it to something like 5% and /proc/sys/vm/dirty_background_ratio to 2%? That would be more appropriate sizing (assuming standard SATA drive). Does it change anything? I can try that but I don't really mind if the kernel uses 2G ram for buffers. The problem is it cannot manage those buffers. Does some kernel structure grow out of proportion when the buffers reach this size or something? Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
On 17 September 2013 23:13, Jan Kara j...@suse.cz wrote: Hello, On Tue 17-09-13 15:31:31, Michal Suchanek wrote: On 5 September 2013 12:12, Michal Suchanek hramr...@gmail.com wrote: On 26 August 2013 15:51, Michal Suchanek hramr...@gmail.com wrote: On 12 March 2013 03:15, Hillf Danton dhi...@gmail.com wrote: On 11 March 2013 13:15, Michal Suchanek hramr...@gmail.com wrote: On 8 February 2013 17:31, Michal Suchanek hramr...@gmail.com wrote: Hello, I am dealing with VM disk images and performing something like wiping free space to prepare image for compressing and storing on server or copying it to external USB disk causes 1) system lockup in order of a few tens of seconds when all CPU cores are 100% used by system and the machine is basicaly unusable 2) oom killer killing processes This all on system with 8G ram so there should be plenty space to work with. This happens with kernels 3.6.4 or 3.7.1 With earlier kernel versions (some 3.0 or 3.2 kernels) this was not a problem even with less ram. I have vm.swappiness = 0 set for a long time already. I did some testing with 3.7.1 and with swappiness as much as 75 the kernel still causes all cores to loop somewhere in system when writing lots of data to disk. With swappiness as much as 90 processes still get killed on large disk writes. Given that the max is 100 the interval in which mm works at all is going to be very narrow, less than 10% of the paramater range. This is a severe regression as is the cpu time consumed by the kernel. The io scheduler is the default cfq. If you have any idea what to try other than downgrading to an earlier unaffected kernel I would like to hear. Can you try commit 3cf23841b4b7(mm/vmscan.c: avoid possible deadlock caused by too_many_isolated())? Or try 3.8 and/or 3.9, additionally? Hello, with deadline IO scheduler I experience this issue less often but it still happens. I am on 3.9.6 Debian kernel so 3.8 did not fix this problem. Do you have some idea what to log so that useful information about the lockup is gathered? This appears to be fixed in vanilla 3.11 kernel. I still get short intermittent lockups and cpu usage spikes up to 20% on a core but nowhere near the minute+ long lockups with all cores 100% on earlier kernels. So I did more testing on the 3.11 kernel and while it works OK with tar you can get severe lockups with mc or kvm. The difference is probably the fact that sane tools do fsync() on files they close forcing the file to write out and the kernel returning possible write errors before they move on to next file. Sorry for chiming in a bit late. But is this really writing to a normal disk? SATA drive or something else? With kvm writing to a file used as virtual disk the system would stall indefinitely until the disk driver in the emulated system would time out, return disk IO error, and the emulated system would stop writing. In top I see all CPU cores 90%+ in wait. System is unusable. With mc the lockups would be indefinite, probably because there is no timeout on writing a file in mc. I tried tuning swappiness and eleveators but the the basic problem is solved by neither: the dirty buffers fill up memory and system stalls trying to resolve the situation. This is really strange. There is /proc/sys/vm/dirty_ratio, which limits amount of dirty memory. By default it is set to 20% of memory which tends to be too much for 8 GB machine. Can you set it to something like 5% and /proc/sys/vm/dirty_background_ratio to 2%? That would be more appropriate sizing (assuming standard SATA drive). Does it change anything? The default for dirty_ratio/dirty_background_ratio is 60/40. Setting these to 5/2 gives about the same result as running the script that syncs every 5s. Setting to 30/10 gives larger data chunks and intermittent lockup before every chunk is written. It is quite possible to set kernel parameters that kill the kernel but 1) this is the default 2) the parameter is set in units that do not prevent the issue in general (% RAM vs #blocks) 3) WTH is the system doing? It's 4core 3GHz cpu so it can handle traversing a structure holding 800M data in the background. Something is seriously rotten somewhere. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
On 9 October 2013 16:19, Michal Suchanek hramr...@gmail.com wrote:
Hello,
On 19 September 2013 12:13, Jan Kara j...@suse.cz wrote:
On Wed 18-09-13 16:56:08, Michal Suchanek wrote:
On 17 September 2013 23:13, Jan Kara j...@suse.cz wrote:
Hello,
The default for dirty_ratio/dirty_background_ratio is 60/40. Setting
Ah, that's not upstream default. Upstream has 20/10. In SLES we use 40/10
to better accomodate some workloads but 60/40 on 8 GB machines with
SATA drive really seems too much. That is going to give memory management a
headache.
The problem is that a good SATA drive can do ~100 MB/s if we are
lucky and IO is sequential. Thus if you have 5 GB of dirty data to write,
it takes 50s at best to write it, with more random IO to image file it can
well take several minutes to write. That may cause some increased latency
when memory reclaim waits for writeback to clean some pages.
these to 5/2 gives about the same result as running the script that
syncs every 5s. Setting to 30/10 gives larger data chunks and
intermittent lockup before every chunk is written.
It is quite possible to set kernel parameters that kill the kernel but
1) this is the default
Not upstream one so you should raise this with Debian I guess. 60/40
looks way out of reasonable range for todays machines.
2) the parameter is set in units that do not prevent the issue in
general (% RAM vs #blocks)
You can set the number of bytes instead of percentage -
/proc/sys/vm/dirty_bytes / dirty_background_bytes. It's just that proper
sizing depends on amount of memory, storage HW, workload. So it's more an
administrative task to set this tunable properly.
3) WTH is the system doing? It's 4core 3GHz cpu so it can handle
traversing a structure holding 800M data in the background. Something
is seriously rotten somewhere.
Likely processes are waiting in direct reclaim for IO to finish. But that
is just guessing. Try running attached script (forgot to attach it to
previous email). You will need systemtap and kernel debuginfo installed.
The script doesn't work with all versions of systemtap (as it is sadly a
moving target) so if it fails, tell me your version of systemtap and I'll
update the script accordingly.
This was fixed for me by the patch posted earlier by Hillf Danton so I
guess this answers what the system was (not) doing:
--- a/mm/vmscan.c Wed Sep 18 08:44:08 2013
+++ b/mm/vmscan.c Wed Sep 18 09:31:34 2013
@@ -1543,8 +1543,11 @@ shrink_inactive_list(unsigned long nr_to
* implies that pages are cycling through the LRU faster than
* they are written so also forcibly stall.
*/
- if (nr_unqueued_dirty == nr_taken || nr_immediate)
+ if (nr_unqueued_dirty == nr_taken || nr_immediate) {
+ if (current_is_kswapd())
+ wakeup_flusher_threads(0, WB_REASON_TRY_TO_FREE_PAGES);
congestion_wait(BLK_RW_ASYNC, HZ/10);
+ }
}
/*
Also 75485363 is hopefully addressing this issue in mainline.
Actually, this was in 3.11 already and it did make the behaviour a bit
better but was not enough.
So is something like the vmscan.c patch going to make it into the
mainline kernel?
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
Hello,
On 19 September 2013 12:13, Jan Kara j...@suse.cz wrote:
On Wed 18-09-13 16:56:08, Michal Suchanek wrote:
On 17 September 2013 23:13, Jan Kara j...@suse.cz wrote:
Hello,
The default for dirty_ratio/dirty_background_ratio is 60/40. Setting
Ah, that's not upstream default. Upstream has 20/10. In SLES we use 40/10
to better accomodate some workloads but 60/40 on 8 GB machines with
SATA drive really seems too much. That is going to give memory management a
headache.
The problem is that a good SATA drive can do ~100 MB/s if we are
lucky and IO is sequential. Thus if you have 5 GB of dirty data to write,
it takes 50s at best to write it, with more random IO to image file it can
well take several minutes to write. That may cause some increased latency
when memory reclaim waits for writeback to clean some pages.
these to 5/2 gives about the same result as running the script that
syncs every 5s. Setting to 30/10 gives larger data chunks and
intermittent lockup before every chunk is written.
It is quite possible to set kernel parameters that kill the kernel but
1) this is the default
Not upstream one so you should raise this with Debian I guess. 60/40
looks way out of reasonable range for todays machines.
2) the parameter is set in units that do not prevent the issue in
general (% RAM vs #blocks)
You can set the number of bytes instead of percentage -
/proc/sys/vm/dirty_bytes / dirty_background_bytes. It's just that proper
sizing depends on amount of memory, storage HW, workload. So it's more an
administrative task to set this tunable properly.
3) WTH is the system doing? It's 4core 3GHz cpu so it can handle
traversing a structure holding 800M data in the background. Something
is seriously rotten somewhere.
Likely processes are waiting in direct reclaim for IO to finish. But that
is just guessing. Try running attached script (forgot to attach it to
previous email). You will need systemtap and kernel debuginfo installed.
The script doesn't work with all versions of systemtap (as it is sadly a
moving target) so if it fails, tell me your version of systemtap and I'll
update the script accordingly.
This was fixed for me by the patch posted earlier by Hillf Danton so I
guess this answers what the system was (not) doing:
--- a/mm/vmscan.c Wed Sep 18 08:44:08 2013
+++ b/mm/vmscan.c Wed Sep 18 09:31:34 2013
@@ -1543,8 +1543,11 @@ shrink_inactive_list(unsigned long nr_to
* implies that pages are cycling through the LRU faster than
* they are written so also forcibly stall.
*/
- if (nr_unqueued_dirty == nr_taken || nr_immediate)
+ if (nr_unqueued_dirty == nr_taken || nr_immediate) {
+ if (current_is_kswapd())
+ wakeup_flusher_threads(0, WB_REASON_TRY_TO_FREE_PAGES);
congestion_wait(BLK_RW_ASYNC, HZ/10);
+ }
}
/*
Also 75485363 is hopefully addressing this issue in mainline.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
Hello On 26 August 2013 15:51, Michal Suchanek hramr...@gmail.com wrote: On 12 March 2013 03:15, Hillf Danton dhi...@gmail.com wrote: On 11 March 2013 13:15, Michal Suchanek hramr...@gmail.com wrote: On 8 February 2013 17:31, Michal Suchanek hramr...@gmail.com wrote: Hello, I am dealing with VM disk images and performing something like wiping free space to prepare image for compressing and storing on server or copying it to external USB disk causes 1) system lockup in order of a few tens of seconds when all CPU cores are 100% used by system and the machine is basicaly unusable 2) oom killer killing processes This all on system with 8G ram so there should be plenty space to work with. This happens with kernels 3.6.4 or 3.7.1 With earlier kernel versions (some 3.0 or 3.2 kernels) this was not a problem even with less ram. I have vm.swappiness = 0 set for a long time already. I did some testing with 3.7.1 and with swappiness as much as 75 the kernel still causes all cores to loop somewhere in system when writing lots of data to disk. With swappiness as much as 90 processes still get killed on large disk writes. Given that the max is 100 the interval in which mm works at all is going to be very narrow, less than 10% of the paramater range. This is a severe regression as is the cpu time consumed by the kernel. The io scheduler is the default cfq. If you have any idea what to try other than downgrading to an earlier unaffected kernel I would like to hear. Can you try commit 3cf23841b4b7(mm/vmscan.c: avoid possible deadlock caused by too_many_isolated())? Or try 3.8 and/or 3.9, additionally? Hello, with deadline IO scheduler I experience this issue less often but it still happens. I am on 3.9.6 Debian kernel so 3.8 did not fix this problem. Do you have some idea what to log so that useful information about the lockup is gathered? This appears to be fixed in vanilla 3.11 kernel. I still get short intermittent lockups and cpu usage spikes up to 20% on a core but nowhere near the minute+ long lockups with all cores 100% on earlier kernels. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: doing lots of disk writes causes oom killer to kill processes
Hello,
On 19 September 2013 10:07, Hillf Danton dhi...@gmail.com wrote:
Hello Michal
Take it easy please, the kernel is made by human hands.
Can you please try the diff(and sorry if mail agent reformats it)?
Best Regards
Hillf
--- a/mm/vmscan.c Wed Sep 18 08:44:08 2013
+++ b/mm/vmscan.c Wed Sep 18 09:31:34 2013
@@ -1543,8 +1543,11 @@ shrink_inactive_list(unsigned long nr_to
* implies that pages are cycling through the LRU faster than
* they are written so also forcibly stall.
*/
- if (nr_unqueued_dirty == nr_taken || nr_immediate)
+ if (nr_unqueued_dirty == nr_taken || nr_immediate) {
+ if (current_is_kswapd())
+ wakeup_flusher_threads(0, WB_REASON_TRY_TO_FREE_PAGES);
congestion_wait(BLK_RW_ASYNC, HZ/10);
+ }
}
/*
--
I applied the patch and raised the dirty block ratios to 30/10 and the
default 60/40 while imaging a VM and did not observe any problems so I
guess this solves it.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] Re: [RFC PATCH 0/9] mtd: nand: add sunxi NAND Flash Controller support
On 13 January 2014 10:02, boris brezillon b.brezil...@overkiz.com wrote: Hi Henrik, On 11/01/2014 22:11, Henrik Nordström wrote: bbrezillon thanks for pointing out your documents bbrezillon I'm trying to get the NAND driver with HW ECC (and HW RND) without using DMA at all I tried many things but did not quite get the ECC reading command to return meaningful resuts. But should work somehow. bbrezillon do you have any other information I could use to do this ? Not really. There is no known code to look at using the nand controller without DMA. All allwinner code uses DMA even the boot ROM (BROM). bbrezillon For example, I wonder why there are 2 RAM sectors (the driver I found only make use of RAM0) I think it's used during DMA to fetch next sector while the previous one is transferred by DMA. But not sure. Some feedback on my tests: - I managed to get HW ECC working without any DMA transfer (using CMD = 01): * I only tested the sequential ECC = ECC are stored between 2 data blocks (1024 byte) * Non sequential ECC should work if I store ECC bytes in the OOB area too (I'll just have to send RANDOM_OUT commands to move to the OOB area before sending the ECC cmd and another RANDOM_OUT to go back to the DATA area) - The HW RND (randomizer) works too, I'll just have to figure out how this could be mainlined: * using a simple dt property to tell the controller it should enable the randomizer * provide an interface (like the nand_ecc_ctrl struct ) for other to add their own randomizer implementation (this was requested: https://lkml.org/lkml/2013/12/13/154) The most complicated part is the boot0 partition. Tell me if I'm wrong, but here's what I understood from your work (and yuq's work too): boot 0 part properties: - uses sequential ECC - uses 1024 bytes ECC blocks - boot0 code is stored only on the first ECC block of each page (1024 bytes + ecc bytes) - boot0 code is stored on the first 64 pages of the first block - boot0 uses HW randomizer with a specific rnd seed (0x4a80) It's not that complicated to read/write from/to boot0, but it's a bit more to mainline this implementation: - the nand chip must use the same ECC algorithm and ECC layout on the whole flash (no partition specific config available) - you cannot mark some part of pages as unused = the nand driver will write the whole page, not just the first ECC block (1024 bytes) I thought about manually creating an mtd device that fullfils these needs (in case we encounter the allwinner,nandn-boot property on a nand@X node), but I'm not sure this is the right approach. Any ideas ? Maybe if varying parameters on one MTD device is not acceptable you could export parts of the flash as different MTD devices each with its own parameters. Since the boot0 part is fixed size this should not really be an issue. Existing MTD drivers that share hardware with other devices exist - eg. the MTD driver which exports part of RAM as MDT device. I wonder if it would be good idea to make it possible to use the NAND only for storage without a boot0 area. If this is selected by a DT parameter as suggested changing the parameter will probably make the NAND unreadable. Thanks Michal Best Regards, Boris Regards Henrik -- You received this message because you are subscribed to the Google Groups linux-sunxi group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] Re: [RFC PATCH 0/9] mtd: nand: add sunxi NAND Flash Controller support
On 29 January 2014 16:43, boris brezillon dev b.brezillon@gmail.com wrote: Hello Michal, On 29/01/2014 16:11, Michal Suchanek wrote: On 13 January 2014 10:02, boris brezillon b.brezil...@overkiz.com wrote: boot 0 part properties: - uses sequential ECC - uses 1024 bytes ECC blocks - boot0 code is stored only on the first ECC block of each page (1024 bytes + ecc bytes) - boot0 code is stored on the first 64 pages of the first block - boot0 uses HW randomizer with a specific rnd seed (0x4a80) It's not that complicated to read/write from/to boot0, but it's a bit more to mainline this implementation: - the nand chip must use the same ECC algorithm and ECC layout on the whole flash (no partition specific config available) - you cannot mark some part of pages as unused = the nand driver will write the whole page, not just the first ECC block (1024 bytes) I thought about manually creating an mtd device that fullfils these needs (in case we encounter the allwinner,nandn-boot property on a nand@X node), but I'm not sure this is the right approach. Any ideas ? Maybe if varying parameters on one MTD device is not acceptable you could export parts of the flash as different MTD devices each with its own parameters. Since the boot0 part is fixed size this should not really be an issue. Existing MTD drivers that share hardware with other devices exist - eg. the MTD driver which exports part of RAM as MDT device. I considered this option (exposing 2 mtd devices which use the same nand chip: one for the boot partition and the other one for the remaining space). I might give it a try. For the moment I'm trying to use standard partitions and then attach one of these partitions as a sunxi-nand-boot-interface. Something similar to what UBI is doing when attaching to an MTD device. This way we can use the NAND as a standard MTD dev and when one partition is attached as a sunxi-nand-boot-interface you can access the boot0 partition using a char dev (/dev/snbi0 ?). The sunxi-nand-boot-interface will provide the appropriate abstraction to hide the specific boot0 layout... What do you think ? If it works with MTD, sure. The problem the two devices avoid is that with uniform parameters across MTD device the boot0 partition is invalid. I wonder if it would be good idea to make it possible to use the NAND only for storage without a boot0 area. If this is selected by a DT parameter as suggested changing the parameter will probably make the NAND unreadable. Actually the NAND controller supports up to 8 chips. I guess only the first one can be used as a boot device. Reserving space for the boot partition on all of these chips is kind of useless. This actually depends on the BROM. I did not read the BROM code so I don't know what it does. Moreover, we can't tell if the user wants to boot from the NAND or from another storage (MMC for example), in this case we don't need to expose the boot0 partition. It's possible to use the NAND only for storage, sure. However, a NAND on which the boo0 area is reserved would be unreadable without reserving boot0 area in the driver, right? The best we can tell is if user specified to reserve the area in the DT. It might be possible to verify the boot0 area the same way BROM does when booting from it. This might be nice option when you don't know what you have on the chip and want to read it but most of the time you will want to enforce bootable or non-bootable format when writing the NAND. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 00/10] net: stmmac: Add sun7i GMAC glue layer
On 6 December 2013 18:29, Chen-Yu Tsai w...@csie.org wrote: Hi, This patch series adds Allwinner sun7i support to stmmac. The Allwinner sun7i SoC A20 integrates an early version of dwmac IP from Synopsys. On top of that is a hardware glue layer. This layer needs to be configured before the dwmac can be used. ... Comments? Thanks, wens Chen-Yu Tsai (10): net: stmmac: Enable stmmac main clock when probing hardware net: stmmac: Honor DT parameter to force DMA store and forward mode net: stmmac: Use platform data tied with compatible strings net: stmmac: sunxi platfrom extensions for GMAC in Allwinner A20 SoC's ARM: dts: sun7i: Add GMAC controller node to sun7i DTSI ARM: dts: sun7i: Add pin muxing options for the GMAC ARM: dts: sun7i: cubietruck: Enable the GMAC ARM: dts: sun7i: cubieboard2: Enable GMAC instead of EMAC ARM: dts: sun7i: olinuxino-micro: Enable GMAC instead of EMAC ARM: dts: sun7i: Add ethernet alias for GMAC Tested-By: Michal Suchanek hramr...@gmail.com Works for me with RGMII and MII phy on top of 3.13rc3. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] Re: [PATCH 3/3] ARM: sunxi: dts: Add ahci support to a few A10 and A20 boards
On 7 December 2013 12:47, Olliver Schinagl oliver+l...@schinagl.nl wrote:
Hey maxime,
On 06-12-13 19:33, Maxime Ripard wrote:
Hi Oliver,
On Wed, Dec 04, 2013 at 01:10:55PM +0100, oli...@schinagl.nl wrote:
From: Oliver Schinagl oli...@schinagl.nl
This patch adds sunxi sata support to A10 and A20 boards that have such
a connector. Some boards also feature a regulator via a GPIO and support
for this is also added.
Signed-off-by: Olliver Schinagl oli...@schinagl.nl
Your git setup seems to be pretty uncertain about how your first name is
spelled :)
I should have formally mention it to confuse less people,
This is how officially my name is spelled (I left out any 'middle' letters.
I never really used it as such, as it confuses people and they always write
it wrong anyway. After years I decided that at least on these patches, I
should write it down properly (googleability etc in the future). So formally
it's Olliver 'oliver' M. Schinagl.
And no, I won't share my middle name :p
There! :)
---
arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 26
+
arch/arm/boot/dts/sun4i-a10.dtsi| 9 +
arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 26
+
arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 26
+
arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 26
+
arch/arm/boot/dts/sun7i-a20.dtsi| 9 +
6 files changed, 122 insertions(+)
Could you split this into several patches please?
Yes, appologies, will take care of this! Sorry,
Oliver
At least one per SoC.
diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
index 425a7db..b620084 100644
--- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
+++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
@@ -42,7 +42,18 @@
};
};
+ sata: ahci@01c18000 {
+ pwr-supply = reg_ahci_5v;
+ status = okay;
+ };
+
pinctrl@01c20800 {
+ ahci_pwr_pin: ahci_pwr_pin@0 {
Please prefix it with name of the board.
+ allwinner,pins = PB8;
+ allwinner,function = gpio_out;
+ allwinner,driver = 0;
+ allwinner,pull = 0;
+ };
Please add a newline here.
led_pins_cubieboard: led_pins@0 {
allwinner,pins = PH20, PH21;
allwinner,function = gpio_out;
@@ -86,4 +97,19 @@
linux,default-trigger = heartbeat;
};
};
+
+ regulators {
+ compatible = simple-bus;
+ pinctrl-names = default;
+
+ reg_ahci_5v: ahci-5v {
+ compatible = regulator-fixed;
+ regulator-name = ahci-5v;
+ regulator-min-microvolt = 500;
+ regulator-max-microvolt = 500;
+ pinctrl-0 = ahci_pwr_pin;
+ gpio = pio 1 8 0;
+ enable-active-high;
+ };
+ };
};
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
b/arch/arm/boot/dts/sun4i-a10.dtsi
index 4dccdb0..53c6cdb 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -306,6 +306,15 @@
#size-cells = 0;
};
+ sata: ahci@01c18000 {
+ compatible = allwinner,sun4i-a10-ahci;
Please use sun4i-ahci for consistency.
+ reg = 0x01c18000 0x1000;
+ interrupts = 0 56 1;
The interrupt here doesn't seem right. Is it actually working at all?
+ clocks = ahb_gates 25, pll6 0;
+ clock-names = ahb_sata, pll6_sata;
+ status = disabled;
+ };
+
intc: interrupt-controller@01c20400 {
compatible = allwinner,sun4i-ic;
reg = 0x01c20400 0x400;
diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
index 5c51cb8..99c5e78 100644
--- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
@@ -34,7 +34,18 @@
};
};
+ sata: ahci@01c18000 {
+ pwr-supply = reg_ahci_5v;
+ status = okay;
+ };
+
pinctrl@01c20800 {
+ ahci_pwr_pin: ahci_pwr_pin@0 {
+ allwinner,pins = PB8;
+
Re: doing lots of disk writes causes oom killer to kill processes
On 9 October 2013 16:19, Michal Suchanek hramr...@gmail.com wrote:
Hello,
On 19 September 2013 12:13, Jan Kara j...@suse.cz wrote:
On Wed 18-09-13 16:56:08, Michal Suchanek wrote:
On 17 September 2013 23:13, Jan Kara j...@suse.cz wrote:
Hello,
The default for dirty_ratio/dirty_background_ratio is 60/40. Setting
Ah, that's not upstream default. Upstream has 20/10. In SLES we use 40/10
to better accomodate some workloads but 60/40 on 8 GB machines with
SATA drive really seems too much. That is going to give memory management a
headache.
The problem is that a good SATA drive can do ~100 MB/s if we are
lucky and IO is sequential. Thus if you have 5 GB of dirty data to write,
it takes 50s at best to write it, with more random IO to image file it can
well take several minutes to write. That may cause some increased latency
when memory reclaim waits for writeback to clean some pages.
these to 5/2 gives about the same result as running the script that
syncs every 5s. Setting to 30/10 gives larger data chunks and
intermittent lockup before every chunk is written.
It is quite possible to set kernel parameters that kill the kernel but
1) this is the default
Not upstream one so you should raise this with Debian I guess. 60/40
looks way out of reasonable range for todays machines.
2) the parameter is set in units that do not prevent the issue in
general (% RAM vs #blocks)
You can set the number of bytes instead of percentage -
/proc/sys/vm/dirty_bytes / dirty_background_bytes. It's just that proper
sizing depends on amount of memory, storage HW, workload. So it's more an
administrative task to set this tunable properly.
3) WTH is the system doing? It's 4core 3GHz cpu so it can handle
traversing a structure holding 800M data in the background. Something
is seriously rotten somewhere.
Likely processes are waiting in direct reclaim for IO to finish. But that
is just guessing. Try running attached script (forgot to attach it to
previous email). You will need systemtap and kernel debuginfo installed.
The script doesn't work with all versions of systemtap (as it is sadly a
moving target) so if it fails, tell me your version of systemtap and I'll
update the script accordingly.
This was fixed for me by the patch posted earlier by Hillf Danton so I
guess this answers what the system was (not) doing:
--- a/mm/vmscan.c Wed Sep 18 08:44:08 2013
+++ b/mm/vmscan.c Wed Sep 18 09:31:34 2013
@@ -1543,8 +1543,11 @@ shrink_inactive_list(unsigned long nr_to
* implies that pages are cycling through the LRU faster than
* they are written so also forcibly stall.
*/
- if (nr_unqueued_dirty == nr_taken || nr_immediate)
+ if (nr_unqueued_dirty == nr_taken || nr_immediate) {
+ if (current_is_kswapd())
+ wakeup_flusher_threads(0, WB_REASON_TRY_TO_FREE_PAGES);
congestion_wait(BLK_RW_ASYNC, HZ/10);
+ }
}
/*
Hello,
Is this being addressed somehow?
It seems the 3.15 kernel still has this issue .. unless it happens to
lock up for some other reason in similar situations.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 29 September 2014 10:54, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 10:27:41AM +0200, Geert Uytterhoeven wrote: Hi Thierry, (CC linux-pm, as PM is the real reason behind disabling unused clocks) (CC gregkh and lkml, for driver core) On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding thierry.red...@gmail.com wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: Quoting Maxime Ripard (2014-09-02 02:25:08) On Fri, Aug 29, 2014 at 04:38:14PM +0200, Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:12:44PM +0200, Maxime Ripard wrote: On Fri, Aug 29, 2014 at 09:01:17AM +0200, Thierry Reding wrote: I would think the memory should still be reserved anyway to make sure nothing else is writing over it. And it's in the device tree anyway because the driver needs to know where to put framebuffer content. So the point I was trying to make is that we can't treat the memory in the same way as clocks because it needs to be explicitly managed. Whereas clocks don't. The driver is simply too generic to know what to do with the clocks. You agreed on the fact that the only thing we need to do with the clocks is claim them. Really, I don't find what's complicated there (or not generic). That's not what I agreed on. What I said is that the only thing we need to do with the clocks is nothing. They are already in the state that they need to be. Claim was probably a poor choice of words, but still. We have to keep the clock running, and both the solution you've been giving and this patch do so in a generic way. It doesn't know what frequency they should be running at We don't care about that. Just like we don't care about which frequency is the memory bus running at. It will just run at whatever frequency is appropriate. Exactly. And you shouldn't have to care about them at all. Firmware has already configured the clocks to run at the correct frequencies, and it has made sure that they are enabled. or what they're used for And we don't care about that either. You're not interested in what output the framebuffer is setup to use, which is pretty much the same here, this is the same thing here. That's precisely what I've been saying. The only thing that simplefb cares about is the memory it should be using and the format of the pixels (and how many of them) it writes into that memory. Everything else is assumed to have been set up. Including clocks. We're really discussing in circles here. Mike? -EHIGHLATENCYRESPONSE I forgot about this thread. Sorry. In an attempt to provide the least helpful answer possible, I will stay clear of all of the stuff relating to how simple should simplefb be and the related reserved memory discussion. A few times in this thread it is stated that we can't prevent unused clocks from being disabled. That is only partially true. The clock framework DOES provide a flag to prevent UNUSED clocks from being disabled at late_initcall-time by a clock garbage collector mechanism. Maxime and others familiar with the clock framework are aware of this. What the framework doesn't do is to allow for a magic flag in DT, in platform_data or elsewhere that says, don't let me get turned off until the right driver claims me. That would be an external or alternative method for preventing a clock from being disabled. We have a method for preventing clocks from being disabled. It is as follows: struct clk *my_clk = clk_get(...); clk_prepare_enable(my_clk); That is how it should be done. Period. With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many It doesn't deal with hardware details for hardware components for which no driver is available (yet). That's why the hardware is still in a usable state, after the firmware has set it up. Clocks, regulators, PM domains typically have system-wide implications, and thus need system-wide drivers (in the absence of such drivers, things would work as-is). Note that the driver still requests resources (ioremap the frame buffer), so it needs to know about that tiny piece of hardware detail. That's not a hardware detail. Or at least it isn't hardware specific. It is needed and the same irrespective of display hardware. Clocks, power domains,
Re: [PATCH 1/3] ARM: dts: sunxi: A10s Olinuxino add missing SPI and simplefb.
Hello,
On 26 April 2015 at 10:39, Maxime Ripard
maxime.rip...@free-electrons.com wrote:
Hi,
On Mon, Mar 23, 2015 at 03:00:31PM +0100, Michal Suchanek wrote:
Signed-off-by: Michal Suchanek hramr...@gmail.com
ohci0 {
status = okay;
};
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi
b/arch/arm/boot/dts/sun5i-a10s.dtsi
index a78c95d..438eba1 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -154,6 +154,20 @@
clocks = apb1_gates 18;
status = disabled;
};
+
+ spi2: spi@01c17000 {
+ compatible = allwinner,sun4i-a10-spi;
+ reg = 0x01c17000 0x1000;
+ interrupts = 12;
+ clocks = ahb_gates 22, spi2_clk;
+ clock-names = ahb, mod;
+ dmas = dma SUN4I_DMA_DEDICATED 29,
+dma SUN4I_DMA_DEDICATED 28;
+ dma-names = rx, tx;
+ status = disabled;
+ #address-cells = 1;
+ #size-cells = 0;
+ };
};
};
This should be in sun5i.dtsi (and in a separate patch).
@@ -198,4 +212,18 @@
allwinner,drive = SUN4I_PINCTRL_30_MA;
allwinner,pull = SUN4I_PINCTRL_NO_PULL;
};
+
+ spi2_pins_a: spi2@0 {
+ allwinner,pins = PB11, PB12, PB13, PB14;
+ allwinner,function = spi2;
+ allwinner,drive = SUN4I_PINCTRL_10_MA;
+ allwinner,pull = SUN4I_PINCTRL_NO_PULL;
+ };
+
+ spi2_pins_b: spi2@1 {
+ allwinner,pins = PE00, PE01, PE02, PE03;
+ allwinner,function = spi2;
+ allwinner,drive = SUN4I_PINCTRL_10_MA;
+ allwinner,pull = SUN4I_PINCTRL_NO_PULL;
+ };
};
Ditto.
A13 does not have these pins so maybe not?
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 26 April 2015 at 16:33, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote: On 26 April 2015 at 14:51, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote: On 26 April 2015 at 13:56, Martin Sperl ker...@martin.sperl.org wrote: On 26.04.2015, at 13:23, Hans de Goede hdego...@redhat.com wrote: I think there is actual a use for just binding spidev as spidev, think e.g. the spi pins on the raspberry pi. How do you deal we suggest with such a situation ? I actually asked the same question a few days ago on the spi list (in thread: spi: spidev: Warn loudly if instantiated from DT as “spidev”) and the summary was: You can still do as before, but you have to accept that long irritating warning. Or you patch spidev.c to include your pattern of choice for compatiblity So the suggestion is to add a compatible string like olimex,uext-slot to spidev and use that compatible in the DT? No, you add a compatible for the device that is connected to the bus through that slot. There is no device connected in the slot by design. The slot is there for connecting random stuff you find in your mailbox or other drawers and boxes. I know. Our point is add a compatible for that random device you find in your mailbox. That would be mailbox,device-tbd I suppose? That can certainly be done but adding a new compatible for every board that has some random pins looks like a needless nuisance to me. Especially compared to i2c where you can just open the bus so long as ti is enabled. Or you implement the following proposal (which needs a volunteer): On 23.04.2015, at 09:42, Geert Uytterhoeven ge...@linux-m68k.org wrote: So what you need is a way to handover from generic spidev to a device-specific driver, cfr. what graphics drivers do when the device has been bound to by vesafb or simplefb. Could this be implemented in a generic way in the spi or DT code? ... On 23.04.2015, at 12:36, Mark Brown broo...@kernel.org wrote: On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote: I guess this has been suggested before: the spi core could provide spidev access to all spi client devices which are not bound by a driver? I don't know if it's been suggested before, certainly nobody did the work to make it happen. I don't think I have a massive objection in principal. Actually, I did it a year ago, and it looked at the time that it wasn't what should be done either. There is nothing like unclaimed device. Either there is a device and driver for it may in principle be loaded later as a module or the chip select is reserved for use from userspace. I never said it was perfect. Userspace driver is valid option and should have the ability to have the chip select reserved. Whether an userspace driver is a valid option can spawn a whole debate by its own, but it's true that we should be able to have it exported to the userspace. However, having a spidev compatible is not the solution to that problem. Having to patch the kernel to use an unknown device with userspace driver is not the answer either. Devices which are not performance critical and don't use existing kernel frameworks don't have any use for kernel driver. In fact, writing a kernel driver for them is counter-productive because you will have to write from scratch when you connect the device to a box running another OS due to interface *and* license difference. Also for driver prototyping you need a compatible which makes the device accessible. If no spidev general compatible is available people will just use comaptible for some random device which happens to bind to spidev and will send many letters of thanks to the DT maintainers when the device used for this purpose suddenly grows a Linux driver. https://lkml.org/lkml/2014/4/28/612 But how do you know there is a device? Devices on i2c can be probed. On spi you just transfer random data and hope it does something useful. Some devices have readable registers and can be probed in a device-specific way but others are write-only. Well, what's the point of communicating with a non-existent device in the first place? I have multitude of SPI devices which are not part of the board and hence its DT and can be connected to the board with jumper wires. Most of them don't have a linux driver or compatible to bind with. Then create such a compatible... I will if and when the device is usable. So binding spidev is in my view just saying that you are going to transfer random data from userspace on this bus. Yes, to a device connected on that bus. Yes, so I have this red rectangular PCB, blue PCB, and red square PCB and blue very thin rectangular PCB. Please enlighten me how to add
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 26 April 2015 at 14:51, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote: On 26 April 2015 at 13:56, Martin Sperl ker...@martin.sperl.org wrote: On 26.04.2015, at 13:23, Hans de Goede hdego...@redhat.com wrote: I think there is actual a use for just binding spidev as spidev, think e.g. the spi pins on the raspberry pi. How do you deal we suggest with such a situation ? I actually asked the same question a few days ago on the spi list (in thread: spi: spidev: Warn loudly if instantiated from DT as “spidev”) and the summary was: You can still do as before, but you have to accept that long irritating warning. Or you patch spidev.c to include your pattern of choice for compatiblity So the suggestion is to add a compatible string like olimex,uext-slot to spidev and use that compatible in the DT? No, you add a compatible for the device that is connected to the bus through that slot. There is no device connected in the slot by design. The slot is there for connecting random stuff you find in your mailbox or other drawers and boxes. That can certainly be done but adding a new compatible for every board that has some random pins looks like a needless nuisance to me. Especially compared to i2c where you can just open the bus so long as ti is enabled. Or you implement the following proposal (which needs a volunteer): On 23.04.2015, at 09:42, Geert Uytterhoeven ge...@linux-m68k.org wrote: So what you need is a way to handover from generic spidev to a device-specific driver, cfr. what graphics drivers do when the device has been bound to by vesafb or simplefb. Could this be implemented in a generic way in the spi or DT code? ... On 23.04.2015, at 12:36, Mark Brown broo...@kernel.org wrote: On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote: I guess this has been suggested before: the spi core could provide spidev access to all spi client devices which are not bound by a driver? I don't know if it's been suggested before, certainly nobody did the work to make it happen. I don't think I have a massive objection in principal. Actually, I did it a year ago, and it looked at the time that it wasn't what should be done either. There is nothing like unclaimed device. Either there is a device and driver for it may in principle be loaded later as a module or the chip select is reserved for use from userspace. Userspace driver is valid option and should have the ability to have the chip select reserved. https://lkml.org/lkml/2014/4/28/612 But how do you know there is a device? Devices on i2c can be probed. On spi you just transfer random data and hope it does something useful. Some devices have readable registers and can be probed in a device-specific way but others are write-only. Well, what's the point of communicating with a non-existent device in the first place? I have multitude of SPI devices which are not part of the board and hence its DT and can be connected to the board with jumper wires. Most of them don't have a linux driver or compatible to bind with. So binding spidev is in my view just saying that you are going to transfer random data from userspace on this bus. Yes, to a device connected on that bus. Yes, so I have this red rectangular PCB, blue PCB, and red square PCB and blue very thin rectangular PCB. Please enlighten me how to add DT bindings for these and the PCB which is in the mail and I did not pick up at the post office yet. Of course, I have some hope that the chips on the PCBs are at least somewhat compatible with what I ordered but I cannot be sure until I test that. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] ARM: sunxi: spi: use proper errno when message is too long.
On 26 April 2015 at 10:42, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sat, Apr 25, 2015 at 09:21:07PM +0200, Michal Suchanek wrote: Signed-off-by: Michal Suchanek hramr...@gmail.com No commit log? --- drivers/spi/spi-sun4i.c | 2 +- drivers/spi/spi-sun6i.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c index fbb0a4d..8238b4e 100644 --- a/drivers/spi/spi-sun4i.c +++ b/drivers/spi/spi-sun4i.c @@ -176,7 +176,7 @@ static int sun4i_spi_transfer_one(struct spi_master *master, /* We don't support transfer larger than the FIFO */ if (tfr-len SUN4I_FIFO_DEPTH) - return -EINVAL; + return -EMSGSIZE; Is it still a thing? The patch to remove such limit hasn't been merged yet? Yes, seems so unless the patches are queued somewhere but not merged yet. reinit_completion(sspi-done); sspi-tx_buf = tfr-tx_buf; diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c index ac48f59..0f5dd91 100644 --- a/drivers/spi/spi-sun6i.c +++ b/drivers/spi/spi-sun6i.c @@ -166,7 +166,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master, /* We don't support transfer larger than the FIFO */ if (tfr-len SUN6I_FIFO_DEPTH) - return -EINVAL; + return -EMSGSIZE; Eventually, we should move to using DMA for these messages longer than the FIFO. I should post these patches. Do you have those in a branch somewhere? Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] spidev: Add DT binding example.
On 26 April 2015 at 12:32, Mark Brown broo...@kernel.org wrote:
On Tue, Mar 24, 2015 at 11:50:53AM +0100, Michal Suchanek wrote:
+A spidev example for devicetree binding in a board dts file
+spi2 {
No, this is broken - nothing should ever bind to spidev as spidev. The
fact that we have a binding document at all is a bug.
And how do you get spidev if nothing binds to it?
When you boot without the binding no device is created.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] spidev: Add DT binding example.
On 26 April 2015 at 13:01, Mark Brown broo...@kernel.org wrote: On Sun, Apr 26, 2015 at 12:54:36PM +0200, Michal Suchanek wrote: On 26 April 2015 at 12:32, Mark Brown broo...@kernel.org wrote: No, this is broken - nothing should ever bind to spidev as spidev. The fact that we have a binding document at all is a bug. And how do you get spidev if nothing binds to it? When you boot without the binding no device is created. Describe your actual hardware in the DT then add the relevant device ID to spidev. My hardware is a sunxi SPI controller and it is described in the DT. I have pins on the board which I want to use for connecting arbitrary devices. How do I add those to spidev other than creating a subnode of the spi controller and binding that to spidev? Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 26 April 2015 at 13:56, Martin Sperl ker...@martin.sperl.org wrote: On 26.04.2015, at 13:23, Hans de Goede hdego...@redhat.com wrote: I think there is actual a use for just binding spidev as spidev, think e.g. the spi pins on the raspberry pi. How do you deal we suggest with such a situation ? I actually asked the same question a few days ago on the spi list (in thread: spi: spidev: Warn loudly if instantiated from DT as “spidev”) and the summary was: You can still do as before, but you have to accept that long irritating warning. Or you patch spidev.c to include your pattern of choice for compatiblity So the suggestion is to add a compatible string like olimex,uext-slot to spidev and use that compatible in the DT? That can certainly be done but adding a new compatible for every board that has some random pins looks like a needless nuisance to me. Especially compared to i2c where you can just open the bus so long as ti is enabled. Or you implement the following proposal (which needs a volunteer): On 23.04.2015, at 09:42, Geert Uytterhoeven ge...@linux-m68k.org wrote: So what you need is a way to handover from generic spidev to a device-specific driver, cfr. what graphics drivers do when the device has been bound to by vesafb or simplefb. Could this be implemented in a generic way in the spi or DT code? ... On 23.04.2015, at 12:36, Mark Brown broo...@kernel.org wrote: On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote: I guess this has been suggested before: the spi core could provide spidev access to all spi client devices which are not bound by a driver? I don't know if it's been suggested before, certainly nobody did the work to make it happen. I don't think I have a massive objection in principal. But how do you know there is a device? Devices on i2c can be probed. On spi you just transfer random data and hope it does something useful. Some devices have readable registers and can be probed in a device-specific way but others are write-only. So binding spidev is in my view just saying that you are going to transfer random data from userspace on this bus. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 26 April 2015 at 17:54, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 05:33:36PM +0200, Michal Suchanek wrote: On 26 April 2015 at 16:33, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote: On 26 April 2015 at 14:51, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote: On 26 April 2015 at 13:56, Martin Sperl ker...@martin.sperl.org wrote: On 26.04.2015, at 13:23, Hans de Goede hdego...@redhat.com wrote: I think there is actual a use for just binding spidev as spidev, think e.g. the spi pins on the raspberry pi. How do you deal we suggest with such a situation ? I actually asked the same question a few days ago on the spi list (in thread: spi: spidev: Warn loudly if instantiated from DT as “spidev”) and the summary was: You can still do as before, but you have to accept that long irritating warning. Or you patch spidev.c to include your pattern of choice for compatiblity So the suggestion is to add a compatible string like olimex,uext-slot to spidev and use that compatible in the DT? No, you add a compatible for the device that is connected to the bus through that slot. There is no device connected in the slot by design. The slot is there for connecting random stuff you find in your mailbox or other drawers and boxes. I know. Our point is add a compatible for that random device you find in your mailbox. That would be mailbox,device-tbd I suppose? If you can find a programming model and a matching datasheet for that device, I suppose, yes. That can certainly be done but adding a new compatible for every board that has some random pins looks like a needless nuisance to me. Especially compared to i2c where you can just open the bus so long as ti is enabled. Or you implement the following proposal (which needs a volunteer): On 23.04.2015, at 09:42, Geert Uytterhoeven ge...@linux-m68k.org wrote: So what you need is a way to handover from generic spidev to a device-specific driver, cfr. what graphics drivers do when the device has been bound to by vesafb or simplefb. Could this be implemented in a generic way in the spi or DT code? ... On 23.04.2015, at 12:36, Mark Brown broo...@kernel.org wrote: On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote: I guess this has been suggested before: the spi core could provide spidev access to all spi client devices which are not bound by a driver? I don't know if it's been suggested before, certainly nobody did the work to make it happen. I don't think I have a massive objection in principal. Actually, I did it a year ago, and it looked at the time that it wasn't what should be done either. There is nothing like unclaimed device. Either there is a device and driver for it may in principle be loaded later as a module or the chip select is reserved for use from userspace. I never said it was perfect. Userspace driver is valid option and should have the ability to have the chip select reserved. Whether an userspace driver is a valid option can spawn a whole debate by its own, but it's true that we should be able to have it exported to the userspace. However, having a spidev compatible is not the solution to that problem. Having to patch the kernel to use an unknown device with userspace driver is not the answer either. Devices which are not performance critical and don't use existing kernel frameworks don't have any use for kernel driver. In fact, writing a kernel driver for them is counter-productive because you will have to write from scratch when you connect the device to a box running another OS due to interface *and* license difference. And here is the debate... Also for driver prototyping you need a compatible which makes the device accessible. If no spidev general compatible is available people will just use compatible for some random device which happens to bind to spidev and will send many letters of thanks to the DT maintainers when the device used for this purpose suddenly grows a Linux driver. If people do dumb things, they should expect it to backfire. Yes, dumb things like not allowing people to say in the DT that the board actually has pins on it connected to a SPI bus. Which is the actual hardware which should be described in the DT. Do you have to describe a modem or terminal emulator in DT to connect it to your serial port? You just describe the port. So here you have a SPI port and it should be described in the DT as faithfully as the serial port. https://lkml.org/lkml/2014/4/28/612 But how do you know there is a device? Devices on i2c can be probed. On spi
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 28 April 2015 at 16:16, Mark Brown broo...@kernel.org wrote: On Tue, Apr 28, 2015 at 02:52:54PM +0200, Michal Suchanek wrote: On 28 April 2015 at 14:15, Eric D. eric.dillm...@gmail.com wrote: I was just seeking a way to make spidev device appear under mainline kernel and found this thread. Could someone explain the right way to do this ? There is no approved RightWay(tm). That is not the case as you well know. As has been said several times the compatible for the device should be added to the match table in spidev.c. That's a way unusable for people who actually want to use spidev so it might be TheApprovedWay(tm) but not RightWay(tm). Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 28 April 2015 at 16:03, Eric D. eric.dillm...@gmail.com wrote: Hi, I give Maxime's patch a try and got 4 spidev devices : /dev/spidev32766.[0-3] root@bpi:~# ls -lh /dev/spidev* crw--- 1 root root 153, 0 Apr 28 15:52 /dev/spidev32766.0 crw--- 1 root root 153, 1 Apr 28 15:52 /dev/spidev32766.1 crw--- 1 root root 153, 2 Apr 28 15:52 /dev/spidev32766.2 crw--- 1 root root 153, 3 Apr 28 15:52 /dev/spidev32766.3 Shouldn't they be numbered from like spidev0.[0-3]. Is this an udev problem ? any clues ? I don't know where the 32766 comes from but it's normal. It seems to be the same number every time for the same port. If you really wanted you could probably rename the device nodes with udev. Or with only one spi bus enabled /dev/spidev*.0 should work too. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 28 April 2015 at 16:12, Maxime Ripard
maxime.rip...@free-electrons.com wrote:
On Tue, Apr 28, 2015 at 07:03:16AM -0700, Eric D. wrote:
Hi,
I give Maxime's patch a try and got 4 spidev devices :
/dev/spidev32766.[0-3]
root@bpi:~# ls -lh /dev/spidev*
crw--- 1 root root 153, 0 Apr 28 15:52 /dev/spidev32766.0
crw--- 1 root root 153, 1 Apr 28 15:52 /dev/spidev32766.1
crw--- 1 root root 153, 2 Apr 28 15:52 /dev/spidev32766.2
crw--- 1 root root 153, 3 Apr 28 15:52 /dev/spidev32766.3
Shouldn't they be numbered from like spidev0.[0-3].
Is this an udev problem ? any clues ?
You're missing an SPI alias in the DT.
Indeed, adding some SPI aliases makes the device nodes tidier:
aliases {
serial0 = uart0;
serial1 = uart2;
serial2 = uart3;
spi0 = spi0;
spi1 = spi1;
spi2 = spi2;
};
However, SPI usage examples I have seen showed some random high number.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 28 April 2015 at 14:15, Eric D. eric.dillm...@gmail.com wrote: Hi, I'am a mainline linux user of A20 (bananapi). I'am currently running a debian jessie with latest mainline kernel (4.0.0+). I have a project of home automation, based on nrfl04+ spi driven wireless chip. I was just seeking a way to make spidev device appear under mainline kernel and found this thread. Could someone explain the right way to do this ? There is no approved RightWay(tm). However, you can use this patch: https://lkml.org/lkml/2014/4/28/612 which is probably the least controversial way for now. The example at the start of the thread also works but is CertainlyNotApprovedWay(tm). HTH Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 28 April 2015 at 19:17, Mark Brown broo...@kernel.org wrote: On Tue, Apr 28, 2015 at 04:22:24PM +0200, Michal Suchanek wrote: On 28 April 2015 at 16:16, Mark Brown broo...@kernel.org wrote: That is not the case as you well know. As has been said several times the compatible for the device should be added to the match table in spidev.c. That's a way unusable for people who actually want to use spidev so it might be TheApprovedWay(tm) but not RightWay(tm). I'm pretty sure that someone who's able to edit one file can edit two. I know you have a viewpoint on this but engaging in this way is not helping anyone. The point is that patching the kernel to use spidev is totally useless complication which is brought on spidev users by obtuse kernel maintainers. This patch applied to a kernel which is distributed with the board solves the problem for all spidev users of the board which TheApprovedWay(tm) does not. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 29 April 2015 at 20:56, Geert Uytterhoeven ge...@linux-m68k.org wrote: On Wed, Apr 29, 2015 at 8:37 PM, Michal Suchanek hramr...@gmail.com wrote: I am using a version of Maxime's patch myself right now. It does not seem it's going to be include in the kernel any time soon, however. FWIW I added the ability to open any CS, even those claimed by kernel drivers. This addresses any potential race of spidev binding before the actual driver but has the potential to introduce some subtle bugs when you open and reconfigure a CS used by a kernel driver or send some commands that upset the device. Uh, that sounds dangerous. Perhaps you can add some safety net, before user space can access them, cfr. /sys/class/gpio/export? That's what accessing random devices from userspace is. I can issue the identify commead to my SPI flash allright since it is idle. I am not sure of its protocol details but I am quite sure that some displays have data transfers that allow pauses so if I sent a command during a screen update it would likely get inserted into the framebuffer bitstream. And changing CS polarity or something like that will certainly have interesting results. Still not binding spidev to busy CS is just one test that can be compiled in as an option. If things stay this simple I don't see much problem with that. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 29 April 2015 at 20:06, Mark Brown broo...@kernel.org wrote: On Wed, Apr 29, 2015 at 07:44:59PM +0200, Michal Suchanek wrote: On 29 April 2015 at 19:40, Mark Brown broo...@kernel.org wrote: Please stop this, it is not helpful. Then please make one of the useful ways of instantiating spidev nodes approved or suggest another that when implemented can be mainlined. I think the rest of the thread had that covered - there's both adding the device IDs and Maxime's patch. And adding device IDs is unacceptable for users of devboards while Maxime's patch is not accepted into the kernel. I am using a version of Maxime's patch myself right now. It does not seem it's going to be include in the kernel any time soon, however. FWIW I added the ability to open any CS, even those claimed by kernel drivers. This addresses any potential race of spidev binding before the actual driver but has the potential to introduce some subtle bugs when you open and reconfigure a CS used by a kernel driver or send some commands that upset the device. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 29 April 2015 at 19:40, Mark Brown broo...@kernel.org wrote: On Tue, Apr 28, 2015 at 10:43:37PM +0200, Michal Suchanek wrote: I know you have a viewpoint on this but engaging in this way is not helping anyone. The point is that patching the kernel to use spidev is totally useless complication which is brought on spidev users by obtuse kernel maintainers. This patch applied to a kernel which is distributed with the board solves the problem for all spidev users of the board which TheApprovedWay(tm) does not. Please stop this, it is not helpful. Then please make one of the useful ways of instantiating spidev nodes approved or suggest another that when implemented can be mainlined. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 26 April 2015 at 17:47, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 04:40:50PM +0200, Hans de Goede wrote: Hi, I've a feeling everyone in this thread is ignoring the raspberry pi use-case. Where the board is specifically designed for educational purposes and used with lots of peripherals which are usually programmed from userspace using e.g. python bindings for i2c-dev or spidev, for such a setup we really want spidev to be loaded on the spibus by default and we really do not have a proper compatible for a child device. I'm not sure we're ignoring it, it just is the exact same use case than the whole spidev use case: people want to write SPI userspace drivers, the rpi really is not special here, except maybe for its user space code base, but it really boils down to the same issue. And no having to use per device devicetree overlays for this is not the answer, this needs to be really really easy. With pre device-tree kernels this just works, we should be able to match that ease of use with devicetree. We do agree on that. We repeatedly told that the DT was not a good solution, overlays or not, and this is exactly one of the reasons. Ok, so how about skipping the bindings altogether. Just instantiate a spidev for each SPI bus and each CS the SPI core knows of once spidev is loaded. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 27 April 2015 at 11:36, Mark Brown broo...@kernel.org wrote: On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote: On 26 April 2015 at 14:51, Maxime Ripard No, you add a compatible for the device that is connected to the bus through that slot. There is no device connected in the slot by design. The slot is there for connecting random stuff you find in your mailbox or other drawers and boxes. You should be using device tree overlays to describe what actually ended up getting connected there. NO -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 27 April 2015 at 12:04, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Sun, Apr 26, 2015 at 08:53:16PM +0200, Michal Suchanek wrote: Also for driver prototyping you need a compatible which makes the device accessible. If no spidev general compatible is available people will just use compatible for some random device which happens to bind to spidev and will send many letters of thanks to the DT maintainers when the device used for this purpose suddenly grows a Linux driver. If people do dumb things, they should expect it to backfire. Yes, dumb things like not allowing people to say in the DT that the board actually has pins on it connected to a SPI bus. Which is the actual hardware which should be described in the DT. It's not connected to an SPI bus. It's connected to a device using an SPI bus. If you just had floating SPI lines, I'm pretty sure you wouldn't care about spidev at all. Do you have to describe a modem or terminal emulator in DT to connect it to your serial port? You just describe the port. So here you have a SPI port and it should be described in the DT as faithfully as the serial port. Except that in the serial port, you have a representation of a bus, while spidev represents a *device* connected on an SPI bus. So these are two different things, really. No it's the same thing, really. With serial you just have serial lines which you expose on a connector. With SPI you have CS so you can technically have several connectors for the same SPI bus selected by different CS. So yes, making a spidev entry for the connector under SPI bus is the equivalent of making an UART entry to specify that there is a connector. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 27 April 2015 at 12:10, Mark Brown broo...@kernel.org wrote: On Sun, Apr 26, 2015 at 04:40:50PM +0200, Hans de Goede wrote: Hi, Please always provide context in your replies so people know what you're talking about. I've a feeling everyone in this thread is ignoring the raspberry pi use-case. Where the board is specifically designed for educational purposes and used with lots of peripherals which are usually programmed from userspace using e.g. python bindings for i2c-dev or spidev, for such a setup we really want spidev to be loaded on the spibus by default and we really do not have a proper compatible for a child device. No, that's a different problem - the context you describe just happens to be your use case but it's in no way universal, there are plenty of expansion boards out there that have devices that use real drivers connected to them normally (and some of those could even be used in the educational contexts you describe). I think the underlying need here is either for a better way of registering things or better tooling around device tree overlays to address the usability issues. No. When you have a serial port and just connect serial device to it with no special requirement you just specify the serial port in DT and talk to the device directly without any DT foo. When there is a BT module with reset lines and a kernel driver you write in DT that you have such and such BT module with such and such reset lines on the uart so the driver picks it up. Same for USB attached on-board WiFi - that some USB device needs special handling does not mean you have to specify your keyboard in DT to connect it to an USB port. What you are mandating here, basically, is equivalent of requiring a DT overlay to connect an USB keyboard or mouse because it is a device connected in hardware to the USB port and DT is supposed to describe the hardware. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 27 April 2015 at 17:13, Geert Uytterhoeven ge...@linux-m68k.org wrote: On Mon, Apr 27, 2015 at 4:28 PM, Michal Suchanek hramr...@gmail.com wrote: When you have a serial port and just connect serial device to it with no special requirement you just specify the serial port in DT and talk to the device directly without any DT foo. When there is a BT module with reset lines and a kernel driver you write in DT that you have such and such BT module with such and such reset lines on the uart so the driver picks it up. Same for USB attached on-board WiFi - that some USB device needs special handling does not mean you have to specify your keyboard in DT to connect it to an USB port. What you are mandating here, basically, is equivalent of requiring a DT overlay to connect an USB keyboard or mouse because it is a device connected in hardware to the USB port and DT is supposed to describe the hardware. Unlike the spi bus, the USB bus is a discoverable bus. So you can probe for connected devices. No need to put them in DT (but you can, if the firmware does the probing and updates the DTB. E.g. Open Firmware did that for PCI (which was not hot-pluggable)). Unlike the spi bus, you can (more or less) find out if a device is attached to a uart, by listening to the bus, and waiting for a start bit. I do agree that it would be nice to put the other end of the uart link in DT, as you can't always know what you're talking to. Is it a modem? Is it a printer? Is it a BT module? Is it Super Grover? Is it Mega Mindy? And the point is that it can be any of those and you can swap them. So the DT cannot and will not describe the other end of the bus. There is some point where the static part of the hardware configuration ends and pluggable devices start and DT has to deal with that. Sticking your head into the sand and pretending there is no connector on the device will only give you a fork. When the DT is unusable for me and it is unusable for other people then either a change gets upstream which would be nice or it's bound to be floating around the net as a patchset which would be annoying but has happened in the past and will again. And we are not really discussing some fundamental change to the kernel that might have unexpected effects on unrelated subsystems. We are just having a bikeshedding debate if and how a SPI connector is represented in DT. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] Using SPI NOR flah on sunxi.
On 30 April 2015 at 18:30, thomas.bet...@rohde-schwarz.com wrote: Hello Michal: I tried to connect a SPI NOR flash to my sunxi board and due to the current sunxi SPI driver limitations it does not work. The SPI driver returns an error when more than 64 bytes are transferred at once due to lack of DMA support. Wouldn't it be easier to fix the SPI driver to handle transfers larger than 64 bytes, filling and draining the FIFO multiple times if neccessary? (As far as I can tell, most SPI drivers do this.) Yes, the intent is to fix this by adding dma support to the driver, eventually. The patch might be still useful for other hardware with developing SPI support. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
Hello, On 3 May 2015 at 23:00, Martin Sperl ker...@martin.sperl.org wrote: On 03.05.2015, at 11:59, Mark Brown broo...@kernel.org wrote: Hrm, yes - that should work. I'd ask Greg, that's not something the bus implements. It is still slightly more “complicated” from a distribution perspective, but if that is what makes it a “clean” solution, then that is the way to go forward. I will investigate the fine details, but I fear we may need some “compatibility” magic similar to “new_id” in USB to make it work, because it seems as if you can ONLY force a driver to bind if it _is_ compatible... See also here: https://lwn.net/Articles/143397/ But from what I can tell this functionality (mentioned in this article by Greg) has not been moved into driver-core and bus, so we would need to run our own version of it. Maybe you could make the spidev driver magically bind to any CS to which no other driver is bound? This would also probably solve the problem with the device going away when the driver is unbound if that still happens. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] Re: [PATCH 0/3] Using SPI NOR flah on sunxi.
Hello, On 1 May 2015 at 14:27, Stefan Monnier monn...@iro.umontreal.ca wrote: The SPI driver returns an error when more than 64 bytes are transferred at once due to lack of DMA support. Have you tried the dmaengine patch and make the SPI driver use it? The dmaengine is already merged or queued in sunxi-wip but the SPI glue fell through the cracks. I found it after digging through the archives so I will see if it works for me. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
On 4 May 2015 at 12:12, Mark Brown broo...@kernel.org wrote: On Sun, May 03, 2015 at 11:00:40PM +0200, Martin Sperl wrote: I will investigate the fine details, but I fear we may need some “compatibility” magic similar to “new_id” in USB to make it work, because it seems as if you can ONLY force a driver to bind if it _is_ compatible... I'm confused. What would the point of the functionality be if not to override the existing data, otherwise we'd already have bound the driver? Presumably you can swap different versions of a driver this way. Many devices have two drivers in Linux (old and new) which are obviously both compatible. Loading driver which is not compatible is something which you probably do not want to be done easily as much as sending random junk to SPI devices controlled by a kernel driver. I am, of course, enjoying the ability to send some ID command to a flash memory which is technically controlled by a kernel driver when I physically replace the chip in the socket or the chip was not seated well to start with and I want to check that it's working without rebooting the board. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.
Hello, On 1 May 2015 at 16:20, Marek Vasut ma...@denx.de wrote: On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: On 1 May 2015 at 01:13, Marek Vasut ma...@denx.de wrote: I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size. http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.pdf page 21: SECTOR_ER (20h) erases 4kByte sector. BLOCK_ER (d8h) erases 64kByte sector. http://www.gigadevice.com/product/download/366.html?locale=en_US page 27-28: Sector Erase (SE) (20h) erases 4kByte sector 64KB Block Erase (BE) (d8h) erases 64kByte sector It's pretty much the same as the datasheet I used http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf It mentions both 32KB Block Erase (BE) (52H) and 64KB Block Erase (BE) (D8H) So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.
On 4 May 2015 at 14:12, Marek Vasut ma...@denx.de wrote: On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: Hello, Hi! On 1 May 2015 at 16:20, Marek Vasut ma...@denx.de wrote: On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: On 1 May 2015 at 01:13, Marek Vasut ma...@denx.de wrote: I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size. http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.p df page 21: SECTOR_ER (20h) erases 4kByte sector. BLOCK_ER (d8h) erases 64kByte sector. http://www.gigadevice.com/product/download/366.html?locale=en_US page 27-28: Sector Erase (SE) (20h) erases 4kByte sector 64KB Block Erase (BE) (d8h) erases 64kByte sector It's pretty much the same as the datasheet I used http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf It mentions both 32KB Block Erase (BE) (52H) and 64KB Block Erase (BE) (D8H) The SPI NOR framework will use 0xbe opcode, no problem. So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though. Which exact part do you refer to please ? Start of page 7 where it says sector size 32/64K in either datasheet. It can refer to both BE opcode variants being supported but it's quite unclear. Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.
On 4 May 2015 at 15:35, Marek Vasut ma...@denx.de wrote: On Monday, May 04, 2015 at 03:18:56 PM, Michal Suchanek wrote: On 4 May 2015 at 14:12, Marek Vasut ma...@denx.de wrote: On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: It mentions both 32KB Block Erase (BE) (52H) and 64KB Block Erase (BE) (D8H) The SPI NOR framework will use 0xbe opcode, no problem. So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though. Which exact part do you refer to please ? Start of page 7 where it says sector size 32/64K in either datasheet. It can refer to both BE opcode variants being supported but it's quite unclear. My guess here would be that the internal organisation of the SPI NOR is in 4k blocks, which is no surprise really. My understanding is that opcode 0x52 erases 8x4k sector (ie. 32k of data) while 0xd8 erases 16x4k sector (ie. 64k of data). I don't see any problem here -- there are two different opcodes which do two different things and their behavior matches the one on various other SPI NORs. Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant. See above. Does it make sense now please ? Yes, makes sense. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.
On 1 May 2015 at 01:13, Marek Vasut ma...@denx.de wrote: On Thursday, April 30, 2015 at 11:13:12 PM, Michal Suchanek wrote: The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks. Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase. Signed-off-by: Michal Suchanek hramr...@gmail.com I _think_ you might be able to determine the size, no ? One way is to ask the vendor, but you can also try something like: 1) erase the whole SPI NOR 2) overwrite it with zeroes (or ones ? I think it should be all ones after erasing). 3) Erase sector 0 4) Read some 128 KiB back 5) Observe what is the difference. I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size. It's a value that we don't use anyway so I just mark it as unknown here for future reference. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] MTD: m25p80: fix write return value.
On 30 April 2015 at 20:43, Marek Vasut ma...@denx.de wrote: On Thursday, April 30, 2015 at 03:33:47 PM, Michal Suchanek wrote: The size of written data was added to user supplied value rather than written at the provided address. You might want to work on the commit message a little, something like the following, but feel free to reword as seen fit. The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented. Otherwise, the patch is correct I believe: Acked-by: Marek Vasut ma...@denx.de ok, I will send an updated version. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/arm: Define xen_arch_suspend()
On 7 May 2015 at 18:55, Boris Ostrovsky boris.ostrov...@oracle.com wrote: Commit 2b953a5e994c (xen: Suspend ticks on all CPUs during suspend) introduced xen_arch_suspend() routine but did so only for x86, breaking ARM builds. We need to add it to ARM as well. Signed-off-by: Boris Ostrovsky boris.ostrov...@oracle.com Reported-by: Michal Suchanek hramr...@gmail.com Tested-by: Stefano Stabellini stefano.stabell...@eu.citrix.com Tested-by: Michal Suchanek hramr...@gmail.com Thanks -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: exynos_defconfig: add options to make wifi usable
The Exynos defconfig includes mwifiex sdio support which is present on some of the Exynos boards. For the WiFi to be usable two extra options are needed. Usermode firmware helper to load out-of-kernel firmware and wireless extensions so the interface can be configured with wireless-tools. Signed-off-by: Michal Suchanek hramr...@gmail.com --- arch/arm/configs/exynos_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig index d034c96..5d4ee83f 100644 --- a/arch/arm/configs/exynos_defconfig +++ b/arch/arm/configs/exynos_defconfig @@ -42,6 +42,7 @@ CONFIG_IP_PNP_BOOTP=y CONFIG_IP_PNP_RARP=y CONFIG_WIRELESS=y CONFIG_CFG80211=y +CONFIG_CFG80211_WEXT=y CONFIG_MWIFIEX=y CONFIG_MWIFIEX_SDIO=y CONFIG_RFKILL_REGULATOR=y @@ -49,6 +50,7 @@ CONFIG_UEVENT_HELPER_PATH=/sbin/hotplug CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y CONFIG_PROC_DEVICETREE=y +CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y CONFIG_DMA_CMA=y CONFIG_CMA_SIZE_MBYTES=64 CONFIG_BLK_DEV_LOOP=y -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: exynos_defconfig: add options to make wifi usable
On 11 May 2015 at 13:25, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello Michal, On 05/11/2015 12:22 PM, Michal Suchanek wrote: The Exynos defconfig includes mwifiex sdio support which is present on some of the Exynos boards. For the WiFi to be usable two extra options are needed. Usermode Your subject line and the commit message are somehow misleading since these options are needed to make the WiFi usable with your current setup. firmware helper to load out-of-kernel firmware and wireless extensions For example, this is only needed if the in-kernel fw loader is not able to find the firmware but isn't needed if the fw is for example in an initial ramdisk and the kernel is able to load it, built in the kernel or if the mwifiex driver is built as a module. I am not sure how is including the firmware in a ramdisk going to improve things over including it in my root filesystem. As far as I am aware it does not make any difference for the kernel. The firmware is not included in the kernel tree nor configured as extra firmware option in the defconfig, either. Otherwise the firmware loader would supposedly find the firmware and we would not have this discussion. The mwifiex driver is configured as built-in in the defconfig so what happens when the driver is built as a module is not relevant for this defconfig. so the interface can be configured with wireless-tools. And wireless extensions is deprecated AFAIK and is only needed for old user-space since most tools should had been converted to use the netlink based CONFIG_CFG80211 interface instead. I'm booting a debian jessie and have WiFi working without CFG80211_WEXT for example. I'm booting Debian Jessie as well and for me WiFi is not working without CFG80211_WEXT for another example. So it might be that some tools have migrated to another interface but at first glance I have no idea what those tools might be in Debian so for me the WiFi is unusable without wireless extensions. That doesn't mean that I'm against your patch (I'm always happy to enable more config options if that makes the defconfig more useful) but the commit message should be accurate about why a change has to be done. Do you have some specific suggestions about improvements to the commit message? Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
problem building on ARM with XEN enabled
Hello, it appears the Linus master tree fails to build on ARM with XEN enabled. Since commit 2b953a5e9 xen: Suspend ticks on all CPUs during suspend provides the suspend function only for x86 this is not surprising. I currently don't use XEN yet but building with XEN enabled was not a problem in the past so this looks like a regression to me. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Add option to bind spidev to all chipselects
On 13 May 2015 at 12:16, Maxime Ripard maxime.rip...@free-electrons.com wrote: Hi, On Wed, May 13, 2015 at 09:34:41AM -, Michal Suchanek wrote: Bypass the check if CS is in use for spidev devices if CONFIG_SPIDEV_SHADOW is set. Rename spidev devices to avoid sysfs conflict. This allows dynamically loading SPI device overlays or communicating with SPI devices configured by a kernel driver from userspace. Signed-off-by: Michal Suchanek hramr...@gmail.com Output from checkpatch: total: 2 errors, 4 warnings, 4 checks, 157 lines checked ... I told you a few times already to run checkpatch before sending your patches, apparently you make a point at ignoring me. Fine. That's a good idea to run, yes. Sorry about that. I also discovered an additional issue with unused variable when the config option is set. That being said, I'm not sure this is the right approach, or at least, it doesn't solve anything. If SPIDEV_SHADOW is not set, you will still have the same issue with addition of new devices on previously unused chip selects, and where we have an spidev device now. What I think we should do is, when a new device is created, we just lookup the modalias of the spi_device associated to it. If that modalias is spidev, then unregister the spidev device, register the new device, you're done. If not, return an error. Yes, that's what I intend to look into eventually. However, this patch is still useful and allows both accessing unused bus with spidev and dynamically loading overlays that would use the bus. On the SPIDEV_SHADOW stuff itself, I'm not sure this is such a good idea. There's a good chance it will break the driver by doing stuff behind its back, possibly in a way that will harm the whole kernel, and it's something we usually try to avoid. What is the possibility to harm the whole kernel? If the kernel crashes because the device misses a message this is somewhat worrying. You could see it as a developer option similar to SCSI error injection and others that can introduce states that would normally occur only rarely. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: Add option to bind spidev to all chipselects
Bypass the check if CS is in use for spidev devices if CONFIG_SPIDEV_SHADOW is
set. Rename spidev devices to avoid sysfs conflict.
This allows dynamically loading SPI device overlays or communicating
with SPI devices configured by a kernel driver from userspace.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/spi/Kconfig | 13 +
drivers/spi/spi.c | 74 ++---
include/linux/spi/spi.h | 1 +
3 files changed, 65 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 198f96b..b477828 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -651,6 +651,19 @@ config SPI_SPIDEV
Note that this application programming interface is EXPERIMENTAL
and hence SUBJECT TO CHANGE WITHOUT NOTICE while it stabilizes.
+config SPIDEV_SHADOW
+ depends on SPI_SPIDEV
+ bool Allow spidev access to configured SPI devices (DANGEROUS)
+ help
+ This creates a spidev device node for every chipselect.
+
+ It is possible to access even SPI devices which are in use by a
+ kernel driver. This allows invoking features not in use by the kernel
+ or checking device state from userspace when the kernel driver fails.
+
+ Sending out-of-order messages to the device or reconfiguring the
+ device might cause driver failure. DANGEROUS
+
config SPI_TLE62X0
tristate Infineon TLE62X0 (for power switching)
depends on SYSFS
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index e6ca46e..b48a0dc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -254,16 +254,23 @@ struct spi_device *spi_alloc_device(struct spi_master
*master)
}
EXPORT_SYMBOL_GPL(spi_alloc_device);
-static void spi_dev_set_name(struct spi_device *spi)
+static void spi_dev_set_name(struct spi_device *spi, const char * alias)
{
struct acpi_device *adev = ACPI_COMPANION(spi-dev);
if (adev) {
- dev_set_name(spi-dev, spi-%s, acpi_dev_name(adev));
+ if (alias)
+ dev_set_name(spi-dev, %s-%s, alias,
acpi_dev_name(adev));
+ else
+ dev_set_name(spi-dev, spi-%s, acpi_dev_name(adev));
return;
}
- dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
+ if (alias)
+ dev_set_name(spi-dev, %s%u.%u, alias, spi-master-bus_num,
+spi-chip_select);
+ else
+ dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
spi-chip_select);
}
@@ -272,22 +279,25 @@ static int spi_dev_check(struct device *dev, void *data)
struct spi_device *spi = to_spi_device(dev);
struct spi_device *new_spi = data;
- if (spi-master == new_spi-master
- spi-chip_select == new_spi-chip_select)
+ if (spi-master == new_spi-master
+spi-chip_select == new_spi-chip_select
+#ifdef CONFIG_SPIDEV_SHADOW
+!spi-shadow !new_spi-shadow
+#endif
+ )
return -EBUSY;
return 0;
}
/**
- * spi_add_device - Add spi_device allocated with spi_alloc_device
+ * spi_add_device_alias - Add spi_device allocated with spi_alloc_device
+ * possibly even when device for the CS exists.
* @spi: spi_device to register
+ * @alias: string used as device name prefix or NULL
*
- * Companion function to spi_alloc_device. Devices allocated with
- * spi_alloc_device can be added onto the spi bus with this function.
- *
- * Returns 0 on success; negative errno on failure
+ * See spi_add_device
*/
-int spi_add_device(struct spi_device *spi)
+static inline int spi_add_device_alias(struct spi_device *spi, const char *
alias)
{
static DEFINE_MUTEX(spi_add_lock);
struct spi_master *master = spi-master;
@@ -303,7 +313,8 @@ int spi_add_device(struct spi_device *spi)
}
/* Set the bus ID string */
- spi_dev_set_name(spi);
+ spi_dev_set_name(spi, alias);
+ spi-shadow = !!alias;
/* We need to make sure there's no other device with this
* chipselect **BEFORE** we call setup(), else we'll trash
@@ -321,15 +332,17 @@ int spi_add_device(struct spi_device *spi)
if (master-cs_gpios)
spi-cs_gpio = master-cs_gpios[spi-chip_select];
- /* Drivers may modify this initial i/o setup, but will
-* normally rely on the device being setup. Devices
-* using SPI_CS_HIGH can't coexist well otherwise...
-*/
- status = spi_setup(spi);
- if (status 0) {
- dev_err(dev, can't setup %s, status %d\n,
- dev_name(spi-dev), status);
- goto done;
+ if (!alias) {
+ /* Drivers may modify this initial i/o setup, but will
+* normally rely on the device being setup. Devices
+* using
Re: [PATCH] spi: Force the registration of the spidev devices
On 13 May 2015 at 13:26, Mark Brown broo...@kernel.org wrote:
On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
While this is nicer than the DT solution because of its accurate hardware
representation, it's still not perfect because you might not have access to
the
DT, or you might be driving a completely generic device (such as a
microcontroller) that might be used for something else in a different
context/board.
Greg, you're copied on this because this seems to be a generic problem
that should perhaps be solved at a driver model level - having a way to
bind userspace access to devices that we don't otherwise have a driver
for. The subsystem could specify the UIO driver to use when no other
driver is available.
Solve this by registering automatically spidev devices for all the unused
chip
selects when a master registers itself against the spi core.
So, aside from the concern about this being generic the other thing here
is that we often have devices offering more chip selects than can
physically be used in a system but not doing anything to ensure that the
invalid ones can't be used. It's unclear to me if that's OK or not, it
might be since it's root only I think but I need to think it through a
bit.
Presumably you could add dt nodes for the chipselects and specify they
are disabled but it does not work for me currently.
This is mostly a cosmetic issue. The chipselects were always there but
now they are visible. Unlike the case when you explicitly bind spidev
using dt node the unusable chipselects are also bound.
This also adds an i2cdev-like feeling, where you get all the spidev devices
all
the time, without any modification.
I2C is a bit safer here here since it's a shared bus so you can't do
anything to devices not connected to the bus by mistake.
This is quite safe too. Unless the device chipselect is activated the
device should ignore whatever you write on the bus - unless you get
the chipselect polarity wrong.
But you can get i2c address and lots of other things wrong too.
+ /*
+ * This is far from perfect since an addition might be
+ * done between here and the call to spi_add_device,
+ * but we can't hold the lock and call spi_add_device
+ * either, as it would trigger a deadlock.
+ *
+ * If such a race occurs, spi_add_device will still
+ * catch it though, as it also checks for devices
+ * being registered several times on the same chip
+ * select.
+ */
+ status = bus_for_each_dev(spi_bus_type, NULL, spi,
+ spi_dev_check);
+ if (status) {
+ dev_dbg(master-dev, Chipselect already in use..
Skipping.);
+ spi_dev_put(spi);
+ continue;
+ }
This still leaves us in the situation where if we do know the device
that is connected we have to explicitly bind it in spidev which is
apparently unreasonably difficult for people. I'm also concerned about
the interactions with DT overlays here - what happens if a DT overlay
or other dynamic hardware instantiation comes along later and does bind
something to this chip select? It seems like we should be able to
combine the two models, and the fact that we only create these devices
with a Kconfig option is a bit of an interesting thing here.
It does not bind anything because the chiselect is busy. That's why I
use a patch which disregards spidev when determining if a chiselect is
busy. That has the problem that you can access devices in use by
kernel using spidev then. Ideally spidev (as checked by module alias
or whatever) would be unbound when other driver requests the
chipselect and rebound when the driver quits.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Force the registration of the spidev devices
On 13 May 2015 at 17:37, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote: On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote: While this is nicer than the DT solution because of its accurate hardware representation, it's still not perfect because you might not have access to the DT, or you might be driving a completely generic device (such as a microcontroller) that might be used for something else in a different context/board. Greg, you're copied on this because this seems to be a generic problem that should perhaps be solved at a driver model level - having a way to bind userspace access to devices that we don't otherwise have a driver for. The subsystem could specify the UIO driver to use when no other driver is available. That doesn't really work. I've been talking to the ACPI people about this, and the problem is don't otherwise have a driver for is an impossible thing to prove, as you never know when a driver is going to be loaded from userspace. Yes, exactly. That's why binding spidev in addition to regular drivers or have spidev bind always and get unbound when another driver tries to bind the CS is preferable. The latter is pretty much specifying an UIO driver to use when no other driver is available in the light of the fact that 'is available' might change over time. You can prove that a driver is not available right now. You can easily bind drivers to devices today from userspace, why not just use the built-in functionality you have today if you know that there is no driver for this hardware. And what exactly do you bind the driver to when there is no device created with the built-in functionality you have today? Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Force the registration of the spidev devices
On 13 May 2015 at 16:36, Mark Brown broo...@kernel.org wrote: On Wed, May 13, 2015 at 02:51:02PM +0200, Maxime Ripard wrote: I'd say we're also ok because if we delegate the device driving logic to userspace, we should expect it to know what it does to first drive the device properly, but also to open the right device for this. What's the worst that could happen in such a case? The data are output without any chipselect line being driven by the controller? Isn't that supposed to be ignored by the devices? I'm more worried about the chip select line being connected to the make the board catch fire signal or whatever (more realistically causing us to drive against some other external component) if the extra chip selects weren't pinmuxed away. They would not be pinmuxed away if 1) they are unused by anything else and cs happens to be the default 2) they are used by a device not in DT right? For the latter case we might want some way to disable unused chipselects as well as for the cosmetic issue of multitude of useless devices popping up. But you know, unused i2c bus can be also connected to make the board catch fire trace and nobody would notice until somebody has the great idea to probe it. Incidentally, both i2c and spi cs are active-low iirc. This still leaves us in the situation where if we do know the device that is connected we have to explicitly bind it in spidev which is apparently unreasonably difficult for people. You can still do that, but the point is that you don't have to. Right, but that's not what I'd expect to happen (and seems to make it easier for people to not list things in the DT at all which doesn't seem great). If we're going to make it available by default I'd expect to be able to use a userspace driver with anything that doesn't have a driver bound rather than with devices that explicitly don't have any identification. Remember that spidev can and is used for boards which have spi *CONNECTOR* to which you can connect some random gadget and use a userspace program to communicate with it. It is cool that you can load a dt overlay if the gadget happens to have a kernel driver. However, what identification do you write in the dt when there is no need to use a kernel driver at all? The option to create a node with 'spidev' compatible was rejected as broken. The option to add new compatible to the spidev driver every time you want to connect a new device was rejected as absurdly complex for end users who have a board with system already preinstalled because it requires adding the compatible in the spidev source and rebuilding the kernel. So this patch implements another option which is to bind spidev *always*. The configuration of the port is then not recorded in the DT and it's up to the user to visually check the port or apply other relevant heuristic and run the correct userspace application. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.
Hello, On 12 May 2015 at 16:27, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Mon, Apr 27, 2015 at 07:07:28PM +0100, Mark Brown wrote: On Mon, Apr 27, 2015 at 07:30:36PM +0200, Maxime Ripard wrote: On Mon, Apr 27, 2015 at 11:16:01AM +0100, Mark Brown wrote: lkml.org is being terrible as usual so I can't see half the thread (or at least got fed up trying to get it to load) A part of it is also here: http://lkml.iu.edu/hypermail/linux/kernel/1405.0/00484.html OK, thanks. but I think the discussion there petered out more than anything else. Maybe it did :) I think so looking at the archives. Or was it the suggestion to make this something that the driver core supported so that we didn't have to open code it for every bus? Probably. That's something I really haven't took the time to look at, and don't really plan on doing so. I guess a good way forward would be to revive this patch, and wait for that generic way to happen. What do you think of this? Probably best, the Pi bootloader does make it a bit more important. Might also be worth speaking to Greg though. So, do you want me to resend that patch and discuss this directly there (with Greg in Cc) ? My general idea is to get overlay loading to work and then make spidev bind to all CS which are not taken by anything and unbind when an overlay tries to take over the CS. Since there are some overlay loading patches available that part can be considered solved but I did not get to looking at the dynamic spidev binding. For now I use your patch with additional patch that marks the spidev devices with a flag so they are not checked when it is determined if the CS is in use. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: exynos_defconfig: add options to make wifi usable
Hello, On 11 May 2015 at 15:28, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: On 05/11/2015 02:23 PM, Michal Suchanek wrote: On 11 May 2015 at 13:25, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello Michal, On 05/11/2015 12:22 PM, Michal Suchanek wrote: The Exynos defconfig includes mwifiex sdio support which is present on some of the Exynos boards. For the WiFi to be usable two extra options are needed. Usermode Your subject line and the commit message are somehow misleading since these options are needed to make the WiFi usable with your current setup. firmware helper to load out-of-kernel firmware and wireless extensions For example, this is only needed if the in-kernel fw loader is not able to find the firmware but isn't needed if the fw is for example in an initial ramdisk and the kernel is able to load it, built in the kernel or if the mwifiex driver is built as a module. I am not sure how is including the firmware in a ramdisk going to improve things over including it in my root filesystem. As far as I am aware it does not make any difference for the kernel. My understanding is that an initial ramdisk is mounted very early in the kernel boot process so even a module-less initamfs that only contains a set of firmwares is enough to make the request_firmware() of built in drivers to succeed. While having those in your root filesystem located in a storage media, will make the request_firmware() to fail since the rootfs is mounted much later after built in drivers' probe functions have already been executed. But now you make me doubt if I'm wrong on my assumptions and I should check if that's the case. I cannot try this myself because u-boot either does not like the initramfs generated by Debian or the initramfs does not fit in the reserved space and is corrupted by subsequently loaded device tree. The firmware is not included in the kernel tree nor configured as extra firmware option in the defconfig, either. Otherwise the firmware loader would supposedly find the firmware and we would not have this discussion. The mwifiex driver is configured as built-in in the defconfig so what happens when the driver is built as a module is not relevant for this defconfig. Yes, these two (fw built-in the kernel and driver as a module) were just other examples of setups that don't require the fallback user-mode helper. At least I would mention in the commit message that given the driver is built-in, no fw is built-in the kernel couldn't be assumed that a initram fs will be used, it is better to enable the user-space fw loading fallback. ok so the interface can be configured with wireless-tools. And wireless extensions is deprecated AFAIK and is only needed for old user-space since most tools should had been converted to use the netlink based CONFIG_CFG80211 interface instead. I'm booting a debian jessie and have WiFi working without CFG80211_WEXT for example. I'm booting Debian Jessie as well and for me WiFi is not working without CFG80211_WEXT for another example. So it might be that some tools have migrated to another interface but at first glance I have no idea what those tools might be in Debian so for me the WiFi is unusable without wireless extensions. I'm using the iw package [0] which according to the package description: will become the canonical command line tool for wireless configuration and iwconfig/wireless-tools will no longer be required Notice the future tense. It's not the case for Jessie. So as far as Debian is concerned wireless extensions are going to be the standard interface for some years to come. So I guess iw uses the new netlink based interface while wireless-tools is still using the old ioctl based API. That's something that could also be mentioned in the commit message. The commit message does mention wireless-tools. I tried to install the wi tool and it indeed does work. However, when managing baroque wireless settings a connection manager is pretty much a requirement. I use wicd which can only use wireless extensions as kernel interface. wpa_supplicant itself can use netlink but the management functions like network listing are done over wext. I also tried installing network-manager because it supposedly works with the new netlink interface. Unfortunately, it does not support connecting to the network I use which uses wpa-enterprise authentication, at least using the nmtui frontend. That doesn't mean that I'm against your patch (I'm always happy to enable more config options if that makes the defconfig more useful) but the commit message should be accurate about why a change has to be done. Do you have some specific suggestions about improvements to the commit message? I already mentioned them above. My point is that the subject and commit message said that WiFi is unusable without these options and that's not the case for all the setups
Re: [PATCH 2/3] MTD: spi-nor: check for short writes in spi_nor_write.
On 21 May 2015 at 01:38, Brian Norris computersforpe...@gmail.com wrote: + linux-spi, Mark On Thu, Apr 30, 2015 at 03:38:50PM +0200, Michal Suchanek wrote: My SPI controller driver does not support DMA so writes are truncated to FIFO size. Which SPI master driver? I am using sunxi SPI driver. The dmaengine support for sunxi is not yet in mainline kernel so the SPI master functionality is limited. Check the amount of data actually written by the driver. I'm not sure if we should just reactively use the retlen, or if we should be communicating such a limitation via the SPI API. Thoughts? Is there any driver that would break if the SPI master truncated writes when the message is too long rather than returning an error an refusing the transfer? m25p80 as is would because it assumes all data is always written. Some display driver I tried earlier has an option to limit transfer size actively in the driver. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] MTD: m25p80: fix write return value.
Hello, On 21 May 2015 at 01:45, Brian Norris computersforpe...@gmail.com wrote: On Thu, Apr 30, 2015 at 03:33:47PM +0200, Michal Suchanek wrote: The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented. v2: clearer commit message Signed-off-by: Michal Suchanek hramr...@gmail.com Acked-by: Marek Vasut ma...@denx.de --- drivers/mtd/devices/m25p80.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 7c8b169..0b2bc21 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -102,7 +102,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len, spi_sync(spi, m); - *retlen += m.actual_length - cmd_sz; + *retlen = m.actual_length - cmd_sz; This is very wrong. It gets a little better once you add your next patches (which are also not good) since those patches reinterpret the usage of retlen, but this one definitely does *not* stand a lone. I'll admit the API is a little odd here, but the callers of this function (see spi_nor_write()) actually depend on calling this multiple times, with the value incrementing each time so we get a summary total. You're now clobbering this value. I'm willing to accept patches to improve this API, if you think that would help. Or to add comments that document this. Yes, the only user of the retlen value ignores it but passes it on so without the following patch this one makes the passed on value different from before. For m25p80 this would be fixed by truncating the write in the driver and setting actual_length appropriately rather than returning an error when the message is too long. It might possibly break other drivers, though. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/11] mtd: ofpart: do not fail probe when no partitions exist
On 4 June 2015 at 17:28, Marek Vasut ma...@denx.de wrote:
On Thursday, June 04, 2015 at 06:54:00 AM, Michal Suchanek wrote:
On 4 June 2015 at 00:58, Marek Vasut ma...@denx.de wrote:
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote:
On Exynos it is necessary to set SPI controller parameters that apply to
a SPI slave in a DT subnode of the slave device. The ofpart code returns
an error when there are subnodes of the SPI flash but no partitions are
found. Change this condition to a warning so that flash without
partitions can be accessed on Exynos.
I have to admit the rationale for this patch is not very clear to me,
sorry. Can you please explain this a bit more ?
This is how the DT entry for SPI slave looks with s3c64xx:
flash: m25p80@0 {
#address-cells = 1;
#size-cells = 1;
compatible = jedec,spi-nor;
reg = 0;
spi-max-frequency = 4000;
linux,max_tx_len = 65536;
SIDENOTE: I thought this was actually added by your patch #8 in this
series. The underscores in the name of the property are not really
consistent with the rest of the names.
m25p,fast-read;
controller-data {
samsung,spi-feedback-delay = 0;
};
};
this is example of flash partitions:
flash@0 {
#address-cells = 1;
#size-cells = 1;
partition@0 {
label = u-boot;
reg = 0x000 0x10;
read-only;
};
uimage@10 {
reg = 0x010 0x20;
};
};
The parser ignores any flash without subnodes and returns 0 (no
partititon). When there is a subnode it assumes the flash is
partitioned and tries to parse the subnodes as partitions. When there
are subnodes and none parses as partition an error is returned. As
shown above it is valid to have subnodes on unpartitioned flash.
When an error is returned from a partition parser the mtdpart code
passes on this error to the flash probe function and the proble of the
flash fails.
What does /proc/mtd tell you when you have no partitions defined
in the DT ? It should provide you with the entire MTD device and
the code shouldn't even try to parse any OF partitions, since you
don't have any.
mtdinfo shows I have no mtd devices and the log shows that probe
failed unless I patch the kernel.
When there is *support* for of partitions the ofparser is run on mtd
probe. If it fails because it considers
controller-data {
samsung,spi-feedback-delay = 0;
};
an invalid partition and does not find any valid partition the probe fails.
There are two problems here
1) the above is not invalid. It just is not a partition. The parser
should not fail seeing this
2) the mtd probe should not fail when a partition parser fails and
should present the unpartitioned device
Both are addressed in separate patches.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/11] mtd: ofpart: do not fail probe when no partitions exist
On 4 June 2015 at 00:58, Marek Vasut ma...@denx.de wrote:
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote:
On Exynos it is necessary to set SPI controller parameters that apply to
a SPI slave in a DT subnode of the slave device. The ofpart code returns
an error when there are subnodes of the SPI flash but no partitions are
found. Change this condition to a warning so that flash without
partitions can be accessed on Exynos.
I have to admit the rationale for this patch is not very clear to me, sorry.
Can you please explain this a bit more ?
This is how the DT entry for SPI slave looks with s3c64xx:
flash: m25p80@0 {
#address-cells = 1;
#size-cells = 1;
compatible = jedec,spi-nor;
reg = 0;
spi-max-frequency = 4000;
linux,max_tx_len = 65536;
m25p,fast-read;
controller-data {
samsung,spi-feedback-delay = 0;
};
};
this is example of flash partitions:
flash@0 {
#address-cells = 1;
#size-cells = 1;
partition@0 {
label = u-boot;
reg = 0x000 0x10;
read-only;
};
uimage@10 {
reg = 0x010 0x20;
};
};
The parser ignores any flash without subnodes and returns 0 (no
partititon). When there is a subnode it assumes the flash is
partitioned and tries to parse the subnodes as partitions. When there
are subnodes and none parses as partition an error is returned. As
shown above it is valid to have subnodes on unpartitioned flash.
When an error is returned from a partition parser the mtdpart code
passes on this error to the flash probe function and the proble of the
flash fails.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 01:03, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. If the data was corrupted you would get corrupted data and not transfer failure. AFAIK there is no checksum or anything. I actually do get corrupted data when I use wrong feedback delay setting. The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. So it is required that the m25p80 driver actively prevents doing transfers that are too large for the current driver state on a particular piece of hardware. Signed-off-by: Michal Suchanek hramr...@gmail.com -- Update commit message and documentation text. --- .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ drivers/mtd/devices/m25p80.c | 24 -- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee681..4e63ae8 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Probably. Some SPI slave drivers already do have similar option. In general it cannot be expected that you can reliably transfer arbitrarily large data over SPI it seems. However, if the nand driver transfers data a page at a time it should be fine. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/11] spi: add more debug prints in s3c64xx
On 4 June 2015 at 12:26, Mark Brown broo...@kernel.org wrote: On Thu, Jun 04, 2015 at 11:33:37AM +0200, Michal Suchanek wrote: On 4 June 2015 at 11:16, Mark Brown broo...@kernel.org wrote: Also for this patch (which just adds some trace) there isn't any clear reason for it to be sent as part of the series at all, it doesn't help deliver the functionality and doesn't depend on the rest of the series. I used this patch to add missing information to dmesg output so I could diagnose the SPI failures so I think it is relevant. The fact that you used this to debug problems is not relevant to any fix you might have developed for those problems; the fix can be explained without needing to know how you got there. Similarly the debugging is hopefully useful in general without needing to know which particular problem prompted it's creation. Yes, this patch should be meaningful on its own. The ultimate purpose of this patch series is to make the SPI NOR flash chip usable on boards of which I have 1 sample. If the results on boards other than mine happen to be different the debug prints will be useful for users of this flash chip. There is no code interdependency with the other patches so if it's preferred I can send patches like this separately. To print a clock name you AFAICT need this header. I think this is an abstraction problem in the clock framework. Fixing the abstraction problem with clock framework would only grow the list of recipients which is already so long you complain about it. This is a bit of a warning sign that the series isn't very focused. It's also just not good practice, sending patches with obvious problems (especially obvious problems that aren't clearly flagged up as something temporary) will frustrate reviewers and can often lead to other issues being obscured. As has been pointed out public interface already exists for getting the clock name so the issue here is cosmetic. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 4 June 2015 at 08:42, Geert Uytterhoeven ge...@linux-m68k.org wrote: On Wed, Jun 3, 2015 at 11:26 PM, Michal Suchanek hramr...@gmail.com wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. This is a driver limitation, not a hardware limitation. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This may be a driver bug. The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. So it is required that the m25p80 driver actively prevents doing transfers that are too large for the current driver state on a particular piece of hardware. OK. DT describes the hardware, not buggy drivers. So IMHO this doesn't belong in DT, but it can be a field in struct spi_master. Unfortunately, this cannot be a field in struct spi_master. The value can and does vary with device configuration and device configuration is only available in DT. For example, on the Snow board one working configuration is 40MHz spi bus speed, feedback delay 0 and maximum transfer size 64k. Another working configuration is 133MHz bus speed, feedback delay 3, and maximum transfer size 128 bytes. You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/11] Enable access to SPI NOR flash on Samsung Snow board
On 4 June 2015 at 00:53, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:39 PM, Michal Suchanek wrote: Hello, Hi, this patch series makes it possible to access the SPI NOR flash in the Samsung XE303 'Snow' Chromebook. Unfortunately not all issues are resolved. To work around an issue with the pl330 dma engine I respun the patch for limiting transfer size in m25p80 driver. Looks like fixing a bug at the wrong place to me ... As the flash does not contain any sane filesystem and is only likely to be accessed with mtd_debug or similar tool the limit of the dma engine is easily reached. Filesystems using shorter data transfers are less likely to be affected. Sounds like the DMA engine driver should be fixed. I looked at the pl330 datasheet and don't see anything obviusly wrong with the pl330 driver in Linux. Ideally it would be fixed but I have no idea what's wrong with it. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/11] spi: add more debug prints in s3c64xx
Hello, On 4 June 2015 at 11:16, Mark Brown broo...@kernel.org wrote: On Wed, Jun 03, 2015 at 09:26:42PM -, Michal Suchanek wrote: The SPI NOR transfers mysteriously fail so add more debug prints about SPI transactions. Please try to only send patches to relevant people - the list of recipients for this is so large that it only barely fits on a single screen in my mail client. Also for this patch (which just adds some trace) there isn't any clear reason for it to be sent as part of the series at all, it doesn't help deliver the functionality and doesn't depend on the rest of the series. I used this patch to add missing information to dmesg output so I could diagnose the SPI failures so I think it is relevant. The failure is reported by the s3c64xx but the root cause is pl330 not finishing a DMA transfer. It is non-obvious that this is the issue. The information that a call to pl330 failed is only available in the s3c64xx driver and not SPI core so I think there is no reasonable way to debug this issue other than adding prints in s3c64xx. This issue is not completely resolved or even well tested (I have only single board to test with) so these prints remain relevant in my view. --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -18,6 +18,7 @@ #include linux/interrupt.h #include linux/delay.h #include linux/clk.h +#include linux/clk-provider.h Whatever you're doing here this indicates that there's a very big abstraction problem going on. I wanted to print a clock name in case the information was relevant for the issue at hand. To print a clock name you AFAICT need this header. I think this is an abstraction problem in the clock framework. Fixing the abstraction problem with clock framework would only grow the list of recipients which is already so long you complain about it. It turns out that in this case the clock was not at fault so I did not need the clock name in the end. + pr_debug(%s %s %s waiting for %ims transferring %zubytes@%iHz, + __func__, sdd-pdev ? dev_name(sdd-pdev-dev) : NULL, + dev_name(sdd-master-dev), + ms, xfer-len, sdd-cur_speed); I'd say dev_dbg() but more generally this is just tracing things that seem to be already covered by the trace points already present in the core, the same goes for most of the rest of it. If there's things missing from the existing trace it seems better to add to it. There is master and slave device involved but it's certainly possible to use dev_dbg on one of them and pass the other as string. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pl330 dma failure
H On 2 June 2015 at 16:17, Michal Suchanek hramr...@gmail.com wrote: On 2 June 2015 at 15:08, Vinod Koul vinod.k...@intel.com wrote: On Sat, May 30, 2015 at 09:37:07PM +0200, Michal Suchanek wrote: Hello, I was trying to read the SPI NOR flash and found that the pl330 controller dma mysteriously fails. Adding Robert, There is the problem that the 256 bytes of dma program buffer does not suffice for the whole of 4M of the flash memory so all of it cannot be possibly transferred in one go with the pl330 driver as is now. However, the practical limit is much smaller. At 1MHz or 40MHz the maximum transfer size that ever finishes is 64k. At 133MHz which is the rated flash memory clock the maximum transfer size that finishes is 128 bytes (only tried power of 2). There is no obvious issue with the DMA program. 128k happens to be the smallest size that requires that the burst instruction is repeated in the cycle and also happens to be the smallest size that fails for any speed I tried. Is there some obvious fix or should I just give up on transferring more than 64k and make the driver truncate any larger transfers? Well dma controllers cannot transfer any size, the controllers do have limitation on maximum transfer per block. So to transfer more data you need to create descriptor with multiple segments (or driver should split that but not done usually) and worst case multiple transfers. Hello, Thanks for your input. As I understand the pl330 it transfers data according to a program executed on the controller. The program itself uses DMALDPS/DMASTA instructions which transfers data one byte at a time and there are up to 2 loops with 8bit counter DMALP. So the theoretical limit by the program buffer size (256 bytes) is about 2Mbytes. However, the program for transferring 128k (which happens to have two DMALDPS/DMASTA instructions in the cycle as opposed to the one instruction for transferring 64k) locks up and never (within 10x the expected time it should take to complete the transfer) reaches DMASEV to signal completion of the DMA transfer. Also subsequent commands on on the flash fail probably due to stale data in some FIFO - this can be resolved by sending lots of nops to the flash. There are two problems with this - it is not known in advance what amount of data can be transferred without pl330 locking up. It is 64k at 40MHz spi and 128bytes at 133MHz spi on my system. The other problem is that even if the limitation was known there is no way to let the upper layer know other than transferring less data than requested. It is also possible to orchestrate many small transactions with DMASEV signalling some pl330 function to program new transfer rather than the user code that waits for the transfer to finish. [ 60.780206] m25p80 spi1.0: from 0x, len 131072 [ 60.780291] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.780297] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 not using dma [ 60.780312] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.780316] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 using dma [ 60.780327] bc041000:DMAMOV CCR 0x804200 [ 60.780333] bc041006:DMAMOV SAR 0x12d3001c [ 60.780338] bc04100c:DMAMOV DAR 0x6802 [ 60.780344] bc041012:DMALP_0 255 [ 60.780349] bc041014:DMALP_1 255 [ 60.780355] bc041016:DMAWFPS 4 [ 60.780360] bc041018:DMALDPS 4 [ 60.780365] bc04101a:DMASTA [ 60.780370] bc04101b:DMAFLUSHP 4 [ 60.780375] bc04101d:DMAWFPS 4 [ 60.780380] bc04101f:DMALDPS 4 [ 60.780385] bc041021:DMASTA [ 60.780390] bc041022:DMAFLUSHP 4 [ 60.780395] bc041024:DMALPENDA_1 bjmpto_e [ 60.780401] bc041026:DMALPENDA_0 bjmpto_12 [ 60.780406] bc041028:DMASEV 0 [ 60.780411] bc04102a:DMAEND [ 60.780423] wait_for_dma 12d3.spi spi1 (null) waiting for 72ms transferring 131072bytes@4000Hz [ 60.903505] wait_for_dma 12d3.spi spi1 (null) waited 125 ms [ 60.903518] m25p80 spi1.0: I/O Error: rx-1 tx-0 res:rx-f tx-p len-131072 [ 60.903530] m25p80 spi1.0: SPI transfer failed: -5 [ 60.903590] spi_master spi1: failed to transfer one message from queue Hello, I added more debug prints (and nops in the middle of the dma program just in case) and clearly the event that is supposed to be produced by the DMASEV in this program is never delivered for program of this length. The shorter program with only one DMALDPS in the loop works. Attaching a dmesg excerpt. It has been pointed out that Catalin Marinas might know of some source of information on this dma controller so adding to CC. Thanks Michal [0.00] Booting Linux on physical CPU 0x0 [0.00] Linux version 4.1.0-rc7-00343-g81371ef (hramrach@iscsi) (gcc version 4.9.2 ( 4.9.2-10) ) #35 SMP PREEMPT Fri Jun 12 12:06:15 CEST 2015 [0.00] CPU
pl330 dma failure
Hello, I was trying to read the SPI NOR flash and found that the pl330 controller dma mysteriously fails. There is the problem that the 256 bytes of dma program buffer does not suffice for the whole of 4M of the flash memory so all of it cannot be possibly transferred in one go with the pl330 driver as is now. However, the practical limit is much smaller. At 1MHz or 40MHz the maximum transfer size that ever finishes is 64k. At 133MHz which is the rated flash memory clock the maximum transfer size that finishes is 128 bytes (only tried power of 2). There is no obvious issue with the DMA program. 128k happens to be the smallest size that requires that the burst instruction is repeated in the cycle and also happens to be the smallest size that fails for any speed I tried. Is there some obvious fix or should I just give up on transferring more than 64k and make the driver truncate any larger transfers? Thanks Michal [ 60.726638] m25p80 spi1.0: from 0x, len 65536 [ 60.726724] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.726730] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 not using dma [ 60.726745] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.726749] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 using dma [ 60.726759] bc041000:DMAMOV CCR 0x804200 [ 60.726765] bc041006:DMAMOV SAR 0x12d3001c [ 60.726770] bc04100c:DMAMOV DAR 0x6801 [ 60.726776] bc041012:DMALP_0 255 [ 60.726782] bc041014:DMALP_1 255 [ 60.726787] bc041016:DMAWFPS 4 [ 60.726792] bc041018:DMALDPS 4 [ 60.726797] bc04101a:DMASTA [ 60.726802] bc04101b:DMAFLUSHP 4 [ 60.726808] bc04101d:DMALPENDA_1 bjmpto_7 [ 60.726813] bc04101f:DMALPENDA_0 bjmpto_b [ 60.726819] bc041021:DMASEV 0 [ 60.726824] bc041023:DMAEND [ 60.726836] wait_for_dma 12d3.spi spi1 (null) waiting for 46ms transferring 65536bytes@4000Hz [ 60.765700] wait_for_dma 12d3.spi spi1 (null) waited 40 ms [ 60.780206] m25p80 spi1.0: from 0x, len 131072 [ 60.780291] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.780297] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 not using dma [ 60.780312] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.780316] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 using dma [ 60.780327] bc041000:DMAMOV CCR 0x804200 [ 60.780333] bc041006:DMAMOV SAR 0x12d3001c [ 60.780338] bc04100c:DMAMOV DAR 0x6802 [ 60.780344] bc041012:DMALP_0 255 [ 60.780349] bc041014:DMALP_1 255 [ 60.780355] bc041016:DMAWFPS 4 [ 60.780360] bc041018:DMALDPS 4 [ 60.780365] bc04101a:DMASTA [ 60.780370] bc04101b:DMAFLUSHP 4 [ 60.780375] bc04101d:DMAWFPS 4 [ 60.780380] bc04101f:DMALDPS 4 [ 60.780385] bc041021:DMASTA [ 60.780390] bc041022:DMAFLUSHP 4 [ 60.780395] bc041024:DMALPENDA_1 bjmpto_e [ 60.780401] bc041026:DMALPENDA_0 bjmpto_12 [ 60.780406] bc041028:DMASEV 0 [ 60.780411] bc04102a:DMAEND [ 60.780423] wait_for_dma 12d3.spi spi1 (null) waiting for 72ms transferring 131072bytes@4000Hz [ 60.903505] wait_for_dma 12d3.spi spi1 (null) waited 125 ms [ 60.903518] m25p80 spi1.0: I/O Error: rx-1 tx-0 res:rx-f tx-p len-131072 [ 60.903530] m25p80 spi1.0: SPI transfer failed: -5 [ 60.903590] spi_master spi1: failed to transfer one message from queue -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pl330 dma failure
On 2 June 2015 at 15:08, Vinod Koul vinod.k...@intel.com wrote: On Sat, May 30, 2015 at 09:37:07PM +0200, Michal Suchanek wrote: Hello, I was trying to read the SPI NOR flash and found that the pl330 controller dma mysteriously fails. Adding Robert, There is the problem that the 256 bytes of dma program buffer does not suffice for the whole of 4M of the flash memory so all of it cannot be possibly transferred in one go with the pl330 driver as is now. However, the practical limit is much smaller. At 1MHz or 40MHz the maximum transfer size that ever finishes is 64k. At 133MHz which is the rated flash memory clock the maximum transfer size that finishes is 128 bytes (only tried power of 2). There is no obvious issue with the DMA program. 128k happens to be the smallest size that requires that the burst instruction is repeated in the cycle and also happens to be the smallest size that fails for any speed I tried. Is there some obvious fix or should I just give up on transferring more than 64k and make the driver truncate any larger transfers? Well dma controllers cannot transfer any size, the controllers do have limitation on maximum transfer per block. So to transfer more data you need to create descriptor with multiple segments (or driver should split that but not done usually) and worst case multiple transfers. Hello, Thanks for your input. As I understand the pl330 it transfers data according to a program executed on the controller. The program itself uses DMALDPS/DMASTA instructions which transfers data one byte at a time and there are up to 2 loops with 8bit counter DMALP. So the theoretical limit by the program buffer size (256 bytes) is about 2Mbytes. However, the program for transferring 128k (which happens to have two DMALDPS/DMASTA instructions in the cycle as opposed to the one instruction for transferring 64k) locks up and never (within 10x the expected time it should take to complete the transfer) reaches DMASEV to signal completion of the DMA transfer. Also subsequent commands on on the flash fail probably due to stale data in some FIFO - this can be resolved by sending lots of nops to the flash. There are two problems with this - it is not known in advance what amount of data can be transferred without pl330 locking up. It is 64k at 40MHz spi and 128bytes at 133MHz spi on my system. The other problem is that even if the limitation was known there is no way to let the upper layer know other than transferring less data than requested. It is also possible to orchestrate many small transactions with DMASEV signalling some pl330 function to program new transfer rather than the user code that waits for the transfer to finish. Thanks Michal -- ~Vinod Thanks Michal [ 60.726638] m25p80 spi1.0: from 0x, len 65536 [ 60.726724] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.726730] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 not using dma [ 60.726745] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.726749] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 using dma [ 60.726759] bc041000:DMAMOV CCR 0x804200 [ 60.726765] bc041006:DMAMOV SAR 0x12d3001c [ 60.726770] bc04100c:DMAMOV DAR 0x6801 [ 60.726776] bc041012:DMALP_0 255 [ 60.726782] bc041014:DMALP_1 255 [ 60.726787] bc041016:DMAWFPS 4 [ 60.726792] bc041018:DMALDPS 4 [ 60.726797] bc04101a:DMASTA [ 60.726802] bc04101b:DMAFLUSHP 4 [ 60.726808] bc04101d:DMALPENDA_1 bjmpto_7 [ 60.726813] bc04101f:DMALPENDA_0 bjmpto_b [ 60.726819] bc041021:DMASEV 0 [ 60.726824] bc041023:DMAEND [ 60.726836] wait_for_dma 12d3.spi spi1 (null) waiting for 46ms transferring 65536bytes@4000Hz [ 60.765700] wait_for_dma 12d3.spi spi1 (null) waited 40 ms [ 60.780206] m25p80 spi1.0: from 0x, len 131072 [ 60.780291] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.780297] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 not using dma [ 60.780312] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 xfer bpw 8 speed 4000 [ 60.780316] s3c64xx_spi_transfer_one 12d3.spi spi1 spi1.0 using dma [ 60.780327] bc041000:DMAMOV CCR 0x804200 [ 60.780333] bc041006:DMAMOV SAR 0x12d3001c [ 60.780338] bc04100c:DMAMOV DAR 0x6802 [ 60.780344] bc041012:DMALP_0 255 [ 60.780349] bc041014:DMALP_1 255 [ 60.780355] bc041016:DMAWFPS 4 [ 60.780360] bc041018:DMALDPS 4 [ 60.780365] bc04101a:DMASTA [ 60.780370] bc04101b:DMAFLUSHP 4 [ 60.780375] bc04101d:DMAWFPS 4 [ 60.780380] bc04101f:DMALDPS 4 [ 60.780385] bc041021:DMASTA [ 60.780390] bc041022:DMAFLUSHP 4 [ 60.780395] bc041024:DMALPENDA_1 bjmpto_e [ 60.780401] bc041026:DMALPENDA_0 bjmpto_12 [ 60.780406] bc041028:DMASEV 0
Re: [PATCH v4 0/7] Add spi-nor SPI transfer error handling
Hello, On 15 August 2015 at 03:51, Bean Huo 霍斌斌 (beanhuo) bean...@micron.com wrote: Hello, with these patches SPI transfer errors are not silently ignored but rather reported to spi-nor users. This should prevent silently dropping data to the floor in cases when the SPI transfer fails and the failure is detected. It has been pointed out that MTD users do not handle the case when data is read only partially so this version adds the last patch which handles this in spi-nor. Thanks Michal Seems parallel nand read/write also has the same condition. I am not familiar with parallel NAND drivers so I have no idea if parallel nand can fail in similar way. As I understand it the parallel nand controller is dedicated piece of hardware just for accessing the nand so there may not be any problems similar to what the generic SPI bus has. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
un4i-ss-cipher.c warning
Hello, when building a kernel with sunxi crypto driver as merged into the sinxi-wip branch I get a compiler warning. I am not sure this is the latest version of the driver. It does not seem to be in mainline yet. Thanks Michal In file included from /scratch/build/linux/include/linux/printk.h:277:0, from /scratch/build/linux/include/linux/kernel.h:13, from /scratch/build/linux/include/linux/clk.h:16, from /scratch/build/linux/drivers/crypto/sunxi-ss/sun4i-ss.h:15, from /scratch/build/linux/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c:17: /scratch/build/linux/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c: In function ‘sun4i_ss_cipher_poll’: /scratch/build/linux/include/linux/dynamic_debug.h:86:3: warning: ‘todo’ may be used uninitialized in this function [-Wmaybe-uninitialized] __dynamic_dev_dbg(descriptor, dev, fmt, \ ^ /scratch/build/linux/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c:144:15: note: ‘todo’ was declared here unsigned int todo; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] dmaengine: pl330: event debug messages.
On 19 August 2015 at 18:41, Vinod Koul vinod.k...@intel.com wrote:
On Tue, Jul 28, 2015 at 10:49:33AM +, Michal Suchanek wrote:
In pl330.c is a define which enables very verbose decoding of genereted
programs.
Also add decoding of thread abort states and signalled events.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/dma/pl330.c | 54
-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 257e0d9..9b02936 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1318,6 +1318,10 @@ static int _setup_req(unsigned dry_run, struct
pl330_thread *thrd,
u8 *buf = req-mc_cpu;
int off = 0;
+ if (!dry_run)
+ dev_dbg(thrd-dmac-ddma.dev,
+ setting up request on thread %i, thrd-id);
+
PL330_DBGMC_START(req-mc_bus);
/* DMAMOV CCR, ccr */
@@ -1545,6 +1549,20 @@ static int pl330_update(struct pl330_dmac *pl330)
pl330-dmac_tbd.reset_mngr = true;
else
pl330-dmac_tbd.reset_mngr = false;
+#ifdef PL330_DEBUG_MCGEN
do we need this here?
It's very verbose so I used the define for verbose debug since there
is one already.
+ if (val) {
+ val = readl(regs + FTM);
+ dev_info(pl330-ddma.dev,
+ Manager thread fault %s%s%s%s%s%s,
+ (val (1 30)) ? dbg iface : sys mem,
+ (val (1 16)) ? fetch error : ,
+ (val (1 5)) ? event sec error : ,
+ (val (1 4)) ? channel sec error : ,
+ (val (1 1)) ? invalid operand : ,
+ (val (1 0)) ? invalid instruction :
+ );
+ }
+#endif
val = readl(regs + FSC) ((1 pl330-pcfg.num_chan) - 1);
pl330-dmac_tbd.reset_chan |= val;
@@ -1552,10 +1570,41 @@ static int pl330_update(struct pl330_dmac *pl330)
int i = 0;
while (i pl330-pcfg.num_chan) {
if (val (1 i)) {
+ u32 fault = readl(regs + FTC(i));
+#ifdef PL330_DEBUG_MCGEN
+ dev_info(pl330-ddma.dev,
+ DMA thread fault
+ %s%s%s%s%s%s%s%s%s%s%s%s,
This break grep! also looks very ugly and should be a dev_debug rather
It looks ugly indeed but I have no idea how to decode flag bitfield
into string in some more sensible way.
If you have an example of nice code doing this please share.
I did not even hit this one since the thread just stalls waiting for
device and does not fault in my case.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 4/9] spi: sun4i: add DMA support
On 20 August 2015 at 16:19, Emilio López emi...@elopez.com.ar wrote: From: Emilio López emi...@elopez.com.ar Something went wrong with overriding the headers Sorry Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hello, On 17 August 2015 at 16:42, Alim Akhtar alim.akh...@gmail.com wrote: HI On Mon, Aug 17, 2015 at 4:56 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/17/2015 02:52 PM, Michal Suchanek wrote: Hello, On 17 August 2015 at 03:55, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi, Michal. On 08/12/2015 09:23 PM, Michal Suchanek wrote: The driver has open-coded test for SDIO cards. Use the mmc core provided MMC_QUIRK_BROKEN_CLK_GATING flag instead. Did you use the clock-gating for SDIO cards? Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken? Could you explain to me more? The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING. I understood your intention. And i read the comment into mmc/core/quirks.c I will test SDIO card with this patch. Thanks. When you test, please check if SDIO IRQ still works, we need to put dw_mmc in low_power mode otherwise SDIO IRQ will be not be generated by dw_mmc host controller. As far as I understand the logic which is removed in this patch and the core logic which replaces it is the same - low power by means of clock gating is *not* enabled for SDIO cards in either case. The original code also checks for SDIO IRQ and disables clock gating regardless of card type which is probably redundant. If not it should be fixed in mmc core. My recent kernel builds which I run on a system with mwifiex card probably include this patch. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hello,
On 17 August 2015 at 03:55, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Michal.
On 08/12/2015 09:23 PM, Michal Suchanek wrote:
The driver has open-coded test for SDIO cards. Use the mmc core provided
MMC_QUIRK_BROKEN_CLK_GATING flag instead.
Did you use the clock-gating for SDIO cards?
Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
Could you explain to me more?
The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING.
It may coincide with MMC_CAP_SDIO_IRQ but that's different flag for
different purpose.
MMC_QUIRK_BROKEN_CLK_GATING is currently set for all SDIO cards except
for cards on a whitelist.
I don't know any particular SDIO card that has problems when the clock
is off and does not use an IRQ.
Thanks
Michal
Best Regards,
Jaehoon Chung
As a bonus this may enable clock gating on SDIO cards that are known to
work with it.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mmc/host/dw_mmc.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 40e9d8e..3bc9fd7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
struct mmc_card *card)
* description of the CLKENA register we should disable low power mode
* for SDIO cards if we need SDIO interrupts to work.
*/
- if (mmc-caps MMC_CAP_SDIO_IRQ) {
- const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
- u32 clk_en_a_old;
- u32 clk_en_a;
+ const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
+ u32 clk_en_a_old;
+ u32 clk_en_a;
- clk_en_a_old = mci_readl(host, CLKENA);
+ clk_en_a_old = mci_readl(host, CLKENA);
- if (card-type == MMC_TYPE_SDIO ||
- card-type == MMC_TYPE_SD_COMBO) {
- set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old ~clken_low_pwr;
- } else {
- clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old | clken_low_pwr;
- }
+ if (card-quirks MMC_QUIRK_BROKEN_CLK_GATING) {
+ set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old ~clken_low_pwr;
+ } else {
+ clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old | clken_low_pwr;
+ }
- if (clk_en_a != clk_en_a_old) {
- mci_writel(host, CLKENA, clk_en_a);
- mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
- SDMMC_CMD_PRV_DAT_WAIT, 0);
- }
+ if (clk_en_a != clk_en_a_old) {
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] of: fdt: mark unflattened tree as detached
The tree returned from of_fdt_unflatten_tree cannot be attached to the
live tree because it is not marked as detached so mark it as such. The
dt resolver checks the flag and refuses to process the tree otherwise.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/of/fdt.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 0749656..32523e6 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -383,7 +383,8 @@ static void * unflatten_dt_node(const void *blob,
*/
static void __unflatten_device_tree(const void *blob,
struct device_node **mynodes,
-void * (*dt_alloc)(u64 size, u64 align))
+void * (*dt_alloc)(u64 size, u64 align),
+bool detached)
{
unsigned long size;
int start;
@@ -427,6 +428,10 @@ static void __unflatten_device_tree(const void *blob,
if (be32_to_cpup(mem + size) != 0xdeadbeef)
pr_warning(End of tree marker overwritten: %08x\n,
be32_to_cpup(mem + size));
+ if (detached) {
+ of_node_set_flag(*mynodes, OF_DETACHED);
+ pr_debug(unflattened tree is detached\n);
+ }
pr_debug( - unflatten_device_tree()\n);
}
@@ -447,7 +452,7 @@ static void *kernel_tree_alloc(u64 size, u64 align)
void of_fdt_unflatten_tree(const unsigned long *blob,
struct device_node **mynodes)
{
- __unflatten_device_tree(blob, mynodes, kernel_tree_alloc);
+ __unflatten_device_tree(blob, mynodes, kernel_tree_alloc, true);
}
EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);
@@ -1099,7 +1104,8 @@ bool __init early_init_dt_scan(void *params)
void __init unflatten_device_tree(void)
{
__unflatten_device_tree(initial_boot_params, of_root,
- early_init_dt_alloc_memory_arch);
+ early_init_dt_alloc_memory_arch,
+ false);
/* Get pointer to /chosen and /aliases nodes for use everywhere */
of_alias_scan(early_init_dt_alloc_memory_arch);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/9] spi: sunxi: check that transfer speed is non-zero
On 20 August 2015 at 16:48, Maxime Ripard maxime.rip...@free-electrons.com wrote: On Thu, Aug 20, 2015 at 02:19:46PM -, Michal Suchanek wrote: When the maximum transfer speed is not set for a SPI slave the value remains 0 and the code in sunxi SPI divides by it. Use an arbitrary speed instead in that case. Signed-off-by: Michal Suchanek hramr...@gmail.com spi-max_speed_hz is set using the spi-max-frequency property, and spi-will error out if that property isn't set. What am I missing? Spidev I guess. You can set the speed to arbitrary value from userspace at any time using the spidev ioctls. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to controller-data
On 29 July 2015 at 16:00, Mark Brown broo...@kernel.org wrote: On Wed, Jul 29, 2015 at 12:19:57PM +0200, Michal Suchanek wrote: Please use subject lines matching the style for the subsytsem so people can spot that the patch is in some way relevant. The controller-data subnode has no compatible. This can lead to other drivers getting confused by it. Add a compatible to make devicetreee unambiguous. I can't tell from this commit message what the issue you're trying to fix is, sorry. Nodes without compatible strings are entirely normal and don't need compatible strings. It sounds like a bug in whatever other driver is becoming confused. The driver that gets confused is ofpart. The two-line patch to allow it to just ignore controller-data has been rejected on the basis that s3c64xx should use a compatible string because ofpart monopolizes all nodes without compatible which are children of a mtd device. Devicetrees containing such nodes that are not partitions are presumably invalid and should be rejected when ofpart is compiled into the kernel. + if (!of_get_property(data_np, compatible, NULL) || + strcmp(of_get_property(data_np, compatible, NULL), +samsung,s3c-controller-data)) + dev_err(spi-dev, child node 'controller-data' does not have correct compatible\n); This will break all existing users which is not acceptable for mainline, we need to preserve compatibility with existing device trees. It will not break anything. It will just spam dmesg. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mtd: ofpart: do not fail probe when no partitions exist
On 27 July 2015 at 22:39, Brian Norris computersforpe...@gmail.com wrote:
On Mon, Jul 27, 2015 at 08:30:43PM -, Michal Suchanek wrote:
...
The controller-data node contains no partition information and no other
subnodes with partition information exist.
The ofpart code returns an error when there are subnodes of the flash DT
node but no partitions are found. This error is then propagated to
mtdpart which propagetes it to MTD probe which fails probing the flash
device.
Change this condition to a warning so that flash without partitions can
be accessed on Exynos with ofpart support compiled in.
You never replied to my suggestion here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/352206.html
Particularly, just define a proper compatibile property for [the
'controller-data'] subnode, and ofpart.c will naturally handle this.
Signed-off-by: Michal Suchanek hramr...@gmail.com
--
- add more verbose explanation
---
drivers/mtd/ofpart.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index aa26c32..a29d29f 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -94,10 +94,10 @@ static int parse_ofpart_partitions(struct mtd_info
*master,
if (!i) {
of_node_put(pp);
- pr_err(No valid partition found on %s\n, node-full_name);
+ pr_warn(No valid partition found on %s\n, node-full_name);
kfree(*pparts);
*pparts = NULL;
- return -EINVAL;
+ return 0;
I don't really like this, since it can turn other invalid device trees
into a silent fallback. I'd really prefer we make it easy to tell the
difference between a MTD partition subnode and another foo-bar subnode.
Ok, so I don't find it reasonable to have one driver handle devicetree
nodes without compatible property and insist that no other driver ever
defines nodes without a compatible property.
So either drivers that parse nodes without a compatible property
should handle nodes that are meant to be used by other drivers or
*every* driver has to use a compatible property including ofpart and
then no collision can happen.
So this goes both ways - either deal with nodes that have no
compatible but are not for your driver or define a compatible for your
driver. The s3c64xx driver is ahead of ofpart here since it just tries
to read a property from a subnode of particular name and falls back to
default if not present.
All in all there is no 'foo-bar devicetree'. The fact that your driver
does not understand a particular part of a devicetree does not mean
the part is incorrect. You cannot know what drivers will emerge in the
future and what bindings will they use. In fact this very issue shows
that you make wrong assumptions about that.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] mtd: spi-nor: Add GD25LQ32C 1.8V SPI NOR flash ID
On 28 July 2015 at 16:33, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 11:07:57 AM, Michal Suchanek wrote:
This 1.8V SPI NOR flash is found on ARM Chromebook XE303C and reads
something like 25LQ32VIG in the middle.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/spi-nor/spi-nor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d78831b..cba3bd0 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -559,6 +559,7 @@ static const struct spi_device_id spi_nor_ids[] = {
/* GigaDevice */
{ gd25q32, INFO(0xc84016, 0, 64 * 1024, 64, SECT_4K) },
+ { gd25lq32c, INFO(0xc86016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25q64, INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
{ gd25q128, INFO(0xc84018, 0, 64 * 1024, 256, SECT_4K) },
Don't we have DT bindings, so we can use jedec,spi-nor prop for all
new SPI NORs without growing this table ?
When jedec,spi-nor compatible is used an attempt is made to identify
the chip using this table.
When the ID is not in the table (or is misread due to transfer error)
spi-nor probe fails.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to controller-data
On 29 July 2015 at 19:16, Mark Brown broo...@kernel.org wrote: On Wed, Jul 29, 2015 at 06:19:24PM +0200, Michal Suchanek wrote: On 29 July 2015 at 16:00, Mark Brown broo...@kernel.org wrote: I can't tell from this commit message what the issue you're trying to fix is, sorry. Nodes without compatible strings are entirely normal and don't need compatible strings. It sounds like a bug in whatever other driver is becoming confused. The driver that gets confused is ofpart. The two-line patch to allow it to just ignore controller-data has been rejected on the basis that s3c64xx should use a compatible string because ofpart monopolizes all nodes without compatible which are children of a mtd device. Devicetrees containing such nodes that are not partitions are presumably invalid and should be rejected when ofpart is compiled into the kernel. That seems like an extremely limited binding, the normal thing here would be to create a specifically named node to contain the collection of subnodes like many PMICs do for their regulators. As a fix I'd suggest just silently ignoring nodes it can't understand, or printing a warning if that's a serious issue. + if (!of_get_property(data_np, compatible, NULL) || + strcmp(of_get_property(data_np, compatible, NULL), +samsung,s3c-controller-data)) + dev_err(spi-dev, child node 'controller-data' does not have correct compatible\n); This will break all existing users which is not acceptable for mainline, we need to preserve compatibility with existing device trees. It will not break anything. It will just spam dmesg. I'm confused - if all this change does is to spam dmesg then what's the point? Presumably when your SPI NOR flash fails to probe this message will be just above and you will look into the binding doc and add the compatible. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On 30 July 2015 at 13:24, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? There is always data. Like in whenever you want to read SPI data you sample the input pin and watever you sample is data so far as SPI bus is concerned. Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Could check that, yes. Maybe I could read the counter in a function which dmaengine client uses to tear down the dma transfer. On a related note I enabled more prints on the spidev test program. I have code that tests maximum transfer size up to the 4k spidev limit and it can transfer full 4k buffer of NOPs at 80MHz. The recieved data is all ones except a 00 at the start. So it looks like read-only transfers fail but full-duplex sort of work. And it reproduces the 00 prefix that was seen in the dma-only setup in otherwise working setup :-S An attempt to read a page of data using the fast-read command fails with timeout. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? No. The DMA driver has no idea if the transfer is at 133MHz, 40MHz, 1MHz, 1kHz, whatever. On the other hand, adding a flush_fifo in the SPI driver after DMA failure allows probing the chip reliably by realoding the driver even just after a failed transfer. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact excellent news. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading
Re: [PATCH v2 1/6] mtd: spi-nor: change return value of read/write
On 3 August 2015 at 23:46, Marek Vasut ma...@denx.de wrote:
On Monday, August 03, 2015 at 08:39:01 PM, Michal Suchanek wrote:
Change the return value of spi-nor device read and write methods to
allow returning amount of data transferred and errors as
read(2)/write(2) does.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
include/linux/mtd/spi-nor.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index e540952..7d782cb 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -185,9 +185,9 @@ struct spi_nor {
int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len,
int write_enable);
- int (*read)(struct spi_nor *nor, loff_t from,
+ ssize_t (*read)(struct spi_nor *nor, loff_t from,
size_t len, size_t *retlen, u_char *read_buf);
- void (*write)(struct spi_nor *nor, loff_t to,
+ ssize_t (*write)(struct spi_nor *nor, loff_t to,
size_t len, size_t *retlen, const u_char *write_buf);
int (*erase)(struct spi_nor *nor, loff_t offs);
You realize that if someone does bisect and has only this patch applied,
the compiler will complain loudly about mismatching data types, right ? :)
Yes, the compiler prints a warning. However, only the return value
which is not used changes so it should not cause any real problem. The
data type in the fsl-quadspi and m25p80 drivers is matched in the
following two patches.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 5/5] mtd: ofpart: move ofpart partitions to a dedicated dt node
On 31 July 2015 at 18:06, Boris Brezillon
boris.brezil...@free-electrons.com wrote:
Hi Michal,
On Thu, 30 Jul 2015 12:10:42 +0200
Michal Suchanek hramr...@gmail.com wrote:
Parsing direct subnodes of a mtd device as partitions is unreliable
since the mtd device is also part of its bus subsystem and can contain
bus data in subnodes.
Move ofpart data to a subnode of its own so it is clear which data is
part of the partition layout.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/ofpart.c | 56
+---
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index aa26c32..2c28aaa 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -29,23 +29,33 @@ static int parse_ofpart_partitions(struct mtd_info
*master,
struct mtd_partition **pparts,
struct mtd_part_parser_data *data)
{
- struct device_node *node;
+ struct device_node *mtd_node;
+ struct device_node *ofpart_node;
const char *partname;
struct device_node *pp;
int nr_parts, i;
+ bool dedicated = true;
if (!data)
return 0;
- node = data-of_node;
- if (!node)
+ mtd_node = data-of_node;
+ if (!mtd_node)
return 0;
+ ofpart_node = of_get_child_by_name(mtd_node, ofpart);
Hm, you should use a more generic name, ofpart of the linux MTD
DT partition parser, but another operating system might decide to name
it otherwise. I think partitions is more appropriate.
Whatever. Presumably some dt maintainer will look at this and figure
out a name that perfectly fits current dt policy.
+ if (!ofpart_node) {
+ pr_warn(%s: 'ofpart' subnode not found on %s. Trying to parse
direct subnodes as partitions.\n,
+ master-name, mtd_node-full_name);
Do we really want to complain here. I mean, a lot of users do not need
to define their partition in a different node.
Yes, we do.
The binding without subnode is considered defective and obsolete.
+ ofpart_node = mtd_node;
+ dedicated = false;
+ }
+
/* First count the subnodes */
nr_parts = 0;
- for_each_child_of_node(node, pp) {
- if (node_has_compatible(pp))
+ for_each_child_of_node(ofpart_node, pp) {
+ if (!dedicated node_has_compatible(pp))
continue;
nr_parts++;
@@ -59,22 +69,36 @@ static int parse_ofpart_partitions(struct mtd_info
*master,
return -ENOMEM;
i = 0;
- for_each_child_of_node(node, pp) {
+ for_each_child_of_node(ofpart_node, pp) {
const __be32 *reg;
int len;
int a_cells, s_cells;
- if (node_has_compatible(pp))
- continue;
+ if (!dedicated node_has_compatible(pp))
+ continue;
Check your indentation (checkpatch should complain here).
reg = of_get_property(pp, reg, len);
if (!reg) {
+ if (dedicated) {
+ pr_debug(%s: ofpart partition %s (%s) missing
reg property.\n,
+ master-name, pp-full_name,
+ mtd_node-full_name);
+ goto ofpart_fail;
+ } else {
nr_parts--;
continue;
Ditto.
Well, it does not complain but the indent is definitely off here.
+ }
}
a_cells = of_n_addr_cells(pp);
s_cells = of_n_size_cells(pp);
+ if (len / 4 != a_cells + s_cells) {
+ pr_debug(%s: ofpart partition %s (%s) error parsing
reg property.\n,
+ master-name, pp-full_name,
+ mtd_node-full_name);
+ goto ofpart_fail;
+ }
+
The above changes have nothing to do with the description you gave in
your commit message.
(*pparts)[i].offset = of_read_number(reg, a_cells);
(*pparts)[i].size = of_read_number(reg + a_cells, s_cells);
@@ -92,15 +116,15 @@ static int parse_ofpart_partitions(struct mtd_info
*master,
i++;
}
- if (!i) {
- of_node_put(pp);
- pr_err(No valid partition found on %s\n, node-full_name);
- kfree(*pparts);
- *pparts = NULL;
- return -EINVAL;
- }
-
Are you sure you can safely remove this check?
Yes. It was incomplete check to reject some partitioning schemes
considered invalid.
Now there is stricter checking above so this can be removed.
Basically the whole point of this patch is to remove this bogus check
Re: [PATCH v2 5/5] mtd: ofpart: move ofpart partitions to a dedicated dt node
On 31 July 2015 at 19:24, Boris Brezillon
boris.brezil...@free-electrons.com wrote:
On Fri, 31 Jul 2015 18:52:01 +0200
Michal Suchanek hramr...@gmail.com wrote:
(*pparts)[i].offset = of_read_number(reg, a_cells);
(*pparts)[i].size = of_read_number(reg + a_cells, s_cells);
@@ -92,15 +116,15 @@ static int parse_ofpart_partitions(struct mtd_info
*master,
i++;
}
- if (!i) {
- of_node_put(pp);
- pr_err(No valid partition found on %s\n, node-full_name);
- kfree(*pparts);
- *pparts = NULL;
- return -EINVAL;
- }
-
Are you sure you can safely remove this check?
Yes. It was incomplete check to reject some partitioning schemes
considered invalid.
Now there is stricter checking above so this can be removed.
Indeed, I was worried about resources deallocation, but this is handle
by the caller, and if nr_parts is zero the master MTD device will
be exposed.
Due to compatibility with the previous scheme there is still
possibility that partitions are allocated, and no partitions are
returned due to the nr_parts--;
So yes, the pparts could possibly leak in this case if there were more
partition parsers following ofpart and should be deallocated.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] mtd: spi-nor: Add GD25LQ32C 1.8V SPI NOR flash ID
On 28 July 2015 at 16:38, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 04:36:29 PM, Michal Suchanek wrote:
On 28 July 2015 at 16:33, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 11:07:57 AM, Michal Suchanek wrote:
This 1.8V SPI NOR flash is found on ARM Chromebook XE303C and reads
something like 25LQ32VIG in the middle.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/spi-nor/spi-nor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c index d78831b..cba3bd0 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -559,6 +559,7 @@ static const struct spi_device_id spi_nor_ids[] = {
/* GigaDevice */
{ gd25q32, INFO(0xc84016, 0, 64 * 1024, 64, SECT_4K) },
+ { gd25lq32c, INFO(0xc86016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25q64, INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
{ gd25q128, INFO(0xc84018, 0, 64 * 1024, 256, SECT_4K) },
Don't we have DT bindings, so we can use jedec,spi-nor prop for all
new SPI NORs without growing this table ?
When jedec,spi-nor compatible is used an attempt is made to identify
the chip using this table.
When the ID is not in the table (or is misread due to transfer error)
spi-nor probe fails.
Dang, I was under the impression we want to avoid growing this table :(
The table not to be grown is the devicetree compatibles table.
jedec,spi-nor should be enough for chips that respond to the ID
command.
BTW jedec is not a registered vendor and adding flash chips to
devicetree causes checkpatch warnings.
Theoretically you could use the ID data to infer block and whole flash
size but code for that is not in the driver. Not sure how many
vendor-specific encodings and exceptions are out there.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] mtd: spi-nor: rework spi nor read and write.
On 28 July 2015 at 20:15, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 11:23:02 AM, Michal Suchanek wrote:
The spi_nor read and write functions pass thru the mtd retlen to the
chip-specific read and write function. This makes it difficult to check
for errors in read and write functions and these errors are not checked.
This leads to silent data corruption.
This patch styles the chip-specific read and write function as unix
read(2) and write(2) and updates the retlen in the spi-nor generic driver.
This also makes it possible to check for write errors.
When pl330 fails to transfer the flash data over SPI I get I/O error
instead of 4M of zeroes.
I do not have sst and fsl hardware to test with.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/devices/m25p80.c | 33 ++-
drivers/mtd/spi-nor/fsl-quadspi.c | 29 ++--
drivers/mtd/spi-nor/spi-nor.c | 57
--- include/linux/mtd/spi-nor.h
| 8 +++---
4 files changed, 81 insertions(+), 46 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index d313f948b..d8f064b 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -75,14 +75,15 @@ static int m25p80_write_reg(struct spi_nor *nor, u8
opcode, u8 *buf, int len, return spi_write(spi, flash-command, len + 1);
}
-static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
- size_t *retlen, const u_char *buf)
+static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
+ const u_char *buf)
{
struct m25p *flash = nor-priv;
struct spi_device *spi = flash-spi;
struct spi_transfer t[2] = {};
struct spi_message m;
int cmd_sz = m25p_cmdsz(nor);
+ ssize_t ret;
spi_message_init(m);
@@ -100,9 +101,14 @@ static void m25p80_write(struct spi_nor *nor, loff_t
to, size_t len, t[1].len = len;
spi_message_add_tail(t[1], m);
- spi_sync(spi, m);
+ ret = spi_sync(spi, m);
+ if (ret)
+ return ret;
- *retlen += m.actual_length - cmd_sz;
+ ret = m.actual_length - cmd_sz;
+ if (ret 0)
+ return -EIO;
+ return ret;
I'd prefer to just add the return value and keep the retlen to keep error
codes and transfer length separate.
I prefer to not pass around retlen because passing it around
- causes code duplication
- makes the code harder to understand
btw. you change the transfer length from unsigned to signed type -- long
transfer might get interpreted as an error.
Note that ssize_t is supposed to be enough for write(2) so when it
does not suffice you have a real system-wide problem.
That aside NOR flash sizes tend to be in the order of megabytes so
this concern is very theoretical.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
Hello, On 4 August 2015 at 19:59, R, Vignesh vigne...@ti.com wrote: On 8/4/2015 9:21 PM, Mark Brown wrote: On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote: @use_mmap_mode: Some SPI controller chips are optimized for interacting with serial flash memories. These chips have memory mapped interface, through which entire serial flash memory slave can be read/written as if though they are physical memories (like RAM). Using this interface, flash can be accessed using memcpy() function and the spi controller hardware will take care of communicating with serial flash over SPI. Setting this flag will indicate the SPI controller driver that the spi_message is from mtd layer to read from/write to flash. The SPI master driver can then appropriately switch the controller to memory mapped interface to read from/write to flash, based on this flag (See drivers/spi/spi-ti-qspi.c for example). NOTE: If the SPI controller chip lacks memory mapped interface, then the driver will ignore this flag and use normal SPI protocol to read from/write to flash. Communication with non-flash SPI devices is not possible using the memory mapped interface. I still can't tell from the above what this interface is supposed to do. It sounds like the use of memory mapped mode is supposed to be transparent to users, it should just affect how the controller interacts with the hardware, but if that's the case why do we need to expose it to users at all? Shouldn't the driver just use memory mapped mode if it's faster? TI QSPI controller has two blocks: 1. SPI_CORE: This is generic(normal) spi mode. This can be used to communicate with any SPI devices (serial flashes as well as non-flash devices like touchscreen). 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only allows reading and writing to an SPI flash device only. Used to speed up flash reads. It _cannot_ be used to communicate with non flash devices. Now, the spi_message that ti-qspi receives in transfer_one() callback can be from mtd device(in which case SFI_MM_IF can be used) or from any other non flash SPI device (in which case SFI_MM_IF must not be used instead SPI_CORE is to be used) but there is no way(is there?) to distinguish where spi_message is from. Therefore I introduced flag (use_mmap_mode) to struct spi_message. mtd driver will set flag to true, this helps the ti-qspi driver to determine that the user is flash device and thus can do read via SFI_MM_IF. If this flag is not set then the user is assumed to be non flash SPI driver and will use SPI_CORE block to communicate. On the whole, I just need a way to determine that the user is a flash device in order to switch to memory mapped interface. Maybe it can be set on the SPI slave rather than each message. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 5 August 2015 at 07:35, Vignesh R vigne...@ti.com wrote: On 08/05/2015 10:51 AM, Michal Suchanek wrote: Hello, On 4 August 2015 at 19:59, R, Vignesh vigne...@ti.com wrote: On 8/4/2015 9:21 PM, Mark Brown wrote: On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote: TI QSPI controller has two blocks: 1. SPI_CORE: This is generic(normal) spi mode. This can be used to communicate with any SPI devices (serial flashes as well as non-flash devices like touchscreen). 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only allows reading and writing to an SPI flash device only. Used to speed up flash reads. It _cannot_ be used to communicate with non flash devices. Now, the spi_message that ti-qspi receives in transfer_one() callback can be from mtd device(in which case SFI_MM_IF can be used) or from any other non flash SPI device (in which case SFI_MM_IF must not be used instead SPI_CORE is to be used) but there is no way(is there?) to distinguish where spi_message is from. Therefore I introduced flag (use_mmap_mode) to struct spi_message. mtd driver will set flag to true, this helps the ti-qspi driver to determine that the user is flash device and thus can do read via SFI_MM_IF. If this flag is not set then the user is assumed to be non flash SPI driver and will use SPI_CORE block to communicate. On the whole, I just need a way to determine that the user is a flash device in order to switch to memory mapped interface. Maybe it can be set on the SPI slave rather than each message. You mean to add flag to spi_device struct? That's ok for me. There are already mode flags so you can just add one more. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/6] mtd: spi-nor: change return value of read/write
On 4 August 2015 at 18:42, Marek Vasut ma...@denx.de wrote:
On Tuesday, August 04, 2015 at 08:42:51 AM, Michal Suchanek wrote:
On 3 August 2015 at 23:46, Marek Vasut ma...@denx.de wrote:
On Monday, August 03, 2015 at 08:39:01 PM, Michal Suchanek wrote:
Change the return value of spi-nor device read and write methods to
allow returning amount of data transferred and errors as
read(2)/write(2) does.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
include/linux/mtd/spi-nor.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index e540952..7d782cb 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -185,9 +185,9 @@ struct spi_nor {
int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len,
int write_enable);
- int (*read)(struct spi_nor *nor, loff_t from,
+ ssize_t (*read)(struct spi_nor *nor, loff_t from,
size_t len, size_t *retlen, u_char *read_buf);
- void (*write)(struct spi_nor *nor, loff_t to,
+ ssize_t (*write)(struct spi_nor *nor, loff_t to,
size_t len, size_t *retlen, const u_char
*write_buf);
int (*erase)(struct spi_nor *nor, loff_t offs);
You realize that if someone does bisect and has only this patch applied,
the compiler will complain loudly about mismatching data types, right ?
:)
Yes, the compiler prints a warning. However, only the return value
which is not used changes so it should not cause any real problem.
Are you certain that ssize_t is equal to int , always , everywhere ? :-)
No, it's larger or equal.
Note that it is possible to do such conversion even without introducing
compiler warnings along the way -- just do the data type conversion first
and the other changes in subsequent patches. It will be clear that one
patch does one thing then.
If this is the only concern with these patches it is easily amended ;-)
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 5 August 2015 at 13:50, Mark Brown broo...@kernel.org wrote: On Tue, Aug 04, 2015 at 11:29:52PM +0530, R, Vignesh wrote: On 8/4/2015 9:21 PM, Mark Brown wrote: On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote: I still can't tell from the above what this interface is supposed to do. It sounds like the use of memory mapped mode is supposed to be transparent to users, it should just affect how the controller interacts with the hardware, but if that's the case why do we need to expose it to users at all? Shouldn't the driver just use memory mapped mode if it's faster? 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only allows reading and writing to an SPI flash device only. Used to speed up flash reads. It _cannot_ be used to communicate with non flash devices. Now, the spi_message that ti-qspi receives in transfer_one() callback can be from mtd device(in which case SFI_MM_IF can be used) or from any other non flash SPI device (in which case SFI_MM_IF must not be used instead SPI_CORE is to be used) but there is no way(is there?) to distinguish where spi_message is from. Therefore I introduced flag (use_mmap_mode) to struct spi_message. mtd driver will set flag to true, this helps the ti-qspi driver to determine that the user is flash device and thus can do read via SFI_MM_IF. If this flag is not set then the user is assumed to be non flash SPI driver and will use SPI_CORE block to communicate. So if you're trying to do this you need to document it adequately so that other people can understand what it is supposed to do and how to use and implement it. People can't really tell how the interface is supposed to work based on what was in the patch and the above isn't really helping. For example, how does this change or restrict what the contents of the spi_message are? On the whole, I just need a way to determine that the user is a flash device in order to switch to memory mapped interface. As far as I can tell you want to set a per spi_message flag saying that the message is a flash read command? If that's what this is trying to do then why do you need to set the flag at all? If the message is in a clearly defined format and it's more efficient to use this mmap mode then surely the driver can just recognise that the format is approprate and switch into mmap mode without being explicitly told - I'm not clear what the flag adds here. ehm, the read command is just one byte. I don't think sending 03 or other random byte as the first byte of a SPI transfer can be used as reliable detection that we are talking to a SPI flash memory. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 5 August 2015 at 14:44, Mark Brown broo...@kernel.org wrote: On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote: On 5 August 2015 at 13:50, Mark Brown broo...@kernel.org wrote: As far as I can tell you want to set a per spi_message flag saying that the message is a flash read command? If that's what this is trying to do then why do you need to set the flag at all? If the message is in a clearly defined format and it's more efficient to use this mmap mode then surely the driver can just recognise that the format is approprate and switch into mmap mode without being explicitly told - I'm not clear what the flag adds here. ehm, the read command is just one byte. I don't think sending 03 or other random byte as the first byte of a SPI transfer can be used as reliable detection that we are talking to a SPI flash memory. Why care - if something is physically in the same format as a flash read command how would a device be able to tell that it wasn't actually a flash read command? The signals sent on the bus are going to be identical anyway. Not only must the command be the same but also the response must be tha same. The flash chip responds by sending arbitrary amount of data. Given that transfer_one gets only the part that sends the read command and the part to do the actual read may or may not follow this is getting a bit hairy. Add in dummy bytes due to fast-read lag and page write wrap-around and you get something that you definitely do not want unless you are really sure that there is a flash memory on the other end of the wire. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to controller-data
On 29 July 2015 at 20:40, Mark Brown broo...@kernel.org wrote: On Wed, Jul 29, 2015 at 08:21:34PM +0200, Michal Suchanek wrote: On 29 July 2015 at 19:16, Mark Brown broo...@kernel.org wrote: It will not break anything. It will just spam dmesg. I'm confused - if all this change does is to spam dmesg then what's the point? Presumably when your SPI NOR flash fails to probe this message will be just above and you will look into the binding doc and add the compatible. If you're looking to add a warning message when the flash device fails to probe then add that in the flash driver when it detects an error, this will cause needless noise for everyone else using this controller purely to work around the broken binding. Technically nobody needs to see the warning with in-tree boards since the dts can be amended with the compatible. There is no error in flash device driver. There is only error parsing partition scheme. In my opinion this should never cause an error. With disk drives failure to parse partition table results in unpartitioned disk. As there are number of partitioning schemes failure to parse one still does not prevent other in succeding. And like I say compatible really seems like it isn't an appropriate property here. So to sum up the discussion adding compatible to s3c64xx controller-data is not desirable and making ofpart more robust is desirable. I think the suggestion to use a subnode for ofpart gives the most robust solution. Even adding compatibles to the partition subnodes ofpart still monopolizes the address and cells property of the mtd node which does not pass the 'if another driver did the same would it work together?' test. So my suggestion is to 1) search of ofpart subnode in mtd node. If present read address and reg from it and search partitions as subnodes of the ofpart node. In this case unknown nodes can cause error. 2) failing that issue a warning and try to parse ofpart partitions from the mtd node itself. In this case unknown nodes cannot cause error for compatibility with other drivers including the already exisitn s3c64xx controller-data node. The parser code can be the same for both cases and only operate on different node with a flag to reject unknown subnodes or not. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 6 August 2015 at 11:02, Mark Brown broo...@kernel.org wrote: On Wed, Aug 05, 2015 at 02:56:09PM +0200, Michal Suchanek wrote: On 5 August 2015 at 14:44, Mark Brown broo...@kernel.org wrote: On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote: I don't think sending 03 or other random byte as the first byte of a SPI transfer can be used as reliable detection that we are talking to a SPI flash memory. Why care - if something is physically in the same format as a flash read command how would a device be able to tell that it wasn't actually a flash read command? The signals sent on the bus are going to be identical anyway. Not only must the command be the same but also the response must be tha same. What difference would that make? The caller is sending a single SPI operation and this is a user visible thing... The flash chip responds by sending arbitrary amount of data. Given that transfer_one gets only the part that sends the read command and the part to do the actual read may or may not follow this is getting a bit hairy. Add in dummy bytes due to fast-read lag and page write wrap-around and you get something that you definitely do not want unless you are really sure that there is a flash memory on the other end of the wire. So if you're doing this you may have a good reason to implement transfer_one_message() instead. Or perhaps implement it in the core and provide operations to do the map and unmap. And of course if this sort of requirement exists that's an obvious thing that must be documented in the interfaces but isn't. We need a lot more thought about the interface here, the lack of any explanation of what the interface is supposed to be and the fact that all questions about it are being answered in terms of describing the specific system are both a bit worrying. Disclaimer: I am not familiar with the hardware for which this patch adds support. However, I am familiar m25p80.c and as I understand it the controller is basically supposed to implement m25p80.c in hardware when this flag is set. If I was using m25p80.c to talk to anything but an actual flash chip it would get me quite worried. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 6 August 2015 at 23:33, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Aug 06, 2015 at 06:14:00PM +0200, Geert Uytterhoeven wrote: On Thu, Aug 6, 2015 at 3:51 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote: On the whole following are my requirements: 1. to be able to communicate with non -flash SPI devices via config port ( this functionality is supported by current driver, I dont want to break it). Or pump any spi_message on to SPI bus directly. 2. take advantage of memory mapped port in order to increase read throughput( and use dma in future) when the slave is a m25p80 type flash. 3. handle m25p80 as well as other slave on multiple chipselects. I just need to know whether the user that requested the transfer is m25p80 driver. If yes, ti-qspi driver can take advantage of memory mapped interface, else just use config port to access SPI bus directly. The problem with this approach is that it's an abomination. It's adding a SPI-user specific hack which is detected by a specific driver. That's really not sane - what happens when we have lots of these kinds of I'm an X SPI-user with drivers detecting that? It's not maintainable in the long term. Yes, your requirements _today_ seem simple and easy, but you're only thinking about today, not tomorrow when you've moved on and someone else has to maintain the mess left behind (or delete it from mainline because they're sick of dealing with a hack.) The spi_message that is received in transfer_one_message() is too generic to imply the slave device that is on the other side of the wire. IMO, the read command does not imply that the slave is m25p80 flash (besides the read opcodes vary across vendors of m25p80 and across modes). I can see both sides of the argument. Mark is saying: if the SPI driver detects that the message to be transmitted is a read command followed by the appropriate number of dummy bytes, and then the data being read _and_ it's using quad-mode access, and the hardware generates _exactly_ that in hardware using the memory mapped mode, there is no reason _not_ to use the hardware to achieve that SPI transaction. The bus activity will be identical to what happens when the SPI controller is used manually to achieve that bus sequence. You're saying: but the documentation says you can't use it for anything except m25p80. If you look at 24.5.4.1.2, it tells you what the SFI generates on the bus, which is: 1. CS active 2. Read command byte sent 3. 1-4 address bytes sent 4. 0-3 dummy bytes sent 5. data bytes read from bus 6. CS inactive So, Mark's point is if we can detect a transaction which fits _that_ bus activity, there's no reason not to use this acceleration for the transaction. What you're failing to counter with is: we don't have enough information in the SPI driver to know how many dummy bytes there are between the address bytes and the data read from the bus. Irrespective of the dummy bytes. What if the spi device is not a FLASH ROM, but some other device, which receives a data packet that accidentally looks like an m25p80 READ command? Well, for the most part it looks like it should still work, but there could be a gotcha, but first, let's get rid of a myth there. The QSPI is _not_ specific to the M25P80. The manual says nothing about being specific to that device. What it says is that it's for SPI NOR memory. It will work with bus widths of 1, 2 or 4 data lines, so it probably works with non-M25P80 SPI NOR devices too - and the fact that the read and write commands are completely programmable suggests that using it with SPI NOR devices which do not use the M25P80 read command value is intended. ... That much is good, but now is the problem - how does the SFI know that we're going to require to read 32 bytes? I think the answer to that is that it doesn't know, so it probably just reads the number of bytes which the access on the SoC bus is asking for, which makes it indeterminant from a software point of view to control how many bytes will be read without provoking another send 0x01, next address, dummy byte sequence. So, I'm now on the side of not parsing commands in the SPI driver, and back on the idea that this needs to be handled in some other manner which doesn't involve polluting the SPI core with flag-hacks. OK, so we can agree that using this hardware acceleration for any kind of transfer indiscriminately is not a very good idea. Now since the description is clearer it's obvious that ti-qspi cannot work fully mmapped as fsl-qspi does because the setup has to be done over normal spi access and using non-m25p80 devices on the same bus is a requirement. The place where it is known if a transfer can use the mmap access is m25p80.c So my suggestion is - add a new method for spi master that
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 6 August 2015 at 13:23, Mark Brown broo...@kernel.org wrote: On Thu, Aug 06, 2015 at 12:01:37PM +0200, Michal Suchanek wrote: However, I am familiar m25p80.c and as I understand it the controller is basically supposed to implement m25p80.c in hardware when this flag is set. But what in concrete terms is that supposed to mean? It's currently just an essentially undocumented flag on a message rather than something operating at the level of a flash chip. That's pretty much where Russell's comments come from. If I was using m25p80.c to talk to anything but an actual flash chip it would get me quite worried. Sure, but at the end of the day it's just emitting standard SPI messages which don't know anything about flash. If those messages are a sensible interface here then why bother with the flag, we can just pattern match on the format of the message. If that doesn't work then probably this isn't a great interface and a separate, application specific interface makes more sense. The messages are sensible interface for communicating with a device that interprets a particular part of the mesasge as address and another particular part of the message as command and sends same amount of junk before reply as the flash chip would. If your device happens to send reply immediately part of it is trashed. If it happens to interpret address differently the data ends up in random part of your memory. So no, that is not something you can autodetect. At the end of the day you have valid SPI messages but the m25p80 layer adds interpretation to those messages which may not always give correct result. On the other hand, if you ever get to m25p80 or spi-nor you can assume any message you send goes to a flash chip and insist that the controller uses the flash-specific interface. If there is possibility of connecting different kind of devices to multiple chipselects on the same master then you probably want to select this option per message or per slave. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 6 August 2015 at 12:22, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Aug 06, 2015 at 12:01:37PM +0200, Michal Suchanek wrote: Disclaimer: I am not familiar with the hardware for which this patch adds support. However, I am familiar m25p80.c and as I understand it the controller is basically supposed to implement m25p80.c in hardware when this flag is set. That, to me, sounds like what you have is: ---m25p80 specific interface---SPI bus---m25p80 device Where the m25p80 specific interface does not expose direct access to the SPI bus? The m25p80 specific hardware interface is presumably optional so you can use it or not. The description is a bit vague, though. In fsl-qspi the driver does not make it optional. I am not sure that controller can be used for non-m25p80 slaves. If that's the case, then maybe you should consider whether using the SPI bus infrastructure is really the best way forward. Would it make more sense instead to adopt a different software structure, something more high-level like: +---+ | m25p80 high-level driver =spi-nor | +--++ | SPI m25p80 driver || +--+| | SPI layer | Special driver=fsl-qspi| +--+| |SPI bus driver|| +--++ | SPI hardware | Special hardware | +--++ Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
mode is set on the controller and
change it as needed before each transfer. This is what is commonly
done for other SPI master hardware configuration options.
On the other end of spectrum could be a solution where the
spi-master driverwould have the opportunity to query the
device-tree for specific propertiesduring the spi_add_device
phase - in this case querying the followingproperty in the
device-tree:
spi-master-XXX,use-mmap-cmd-mode = 0x08 0x38;
to implement mmap-mode for commands 0x08 and 0x38.
This is bogus.
Firstly this is per-slave and not per-master.
Secondly in devicetree you have jedec,spi-nor which says exactly that
the SPI slave is a device that responds to the JEDEC identify command.
Nothing more. The data returned from the identify command is used to
determine what the command opcodes are, what features are supported,
etc. So that is *not* in the dt and there is no place for hardwiring
opcodes in the dt.
Alternatively we could also introduce generic alternate modes
for the driver(similar to GPIO - ALT modes), but that would be
less transparent and more hard-coded...
In the end this would mean that from the nor framework side
therewould be no change at all - it still would be issuing
something like this:
/* send the normal block read command */
t[0].tx_buf = flash-command;
/* note that the address would be encoded here */
t[0].len = m25p_cmdsz(nor);
spi_message_add_tail(t[0], m);
/* dummy bytes could also get added to the above transfer */
t[1].tx_buf = dummy_buffer;
t[1].len = dummy;
spi_message_add_tail(t[1], m);
/* the real transfer */
t[2].rx_buf = read_buffer;
t[2].len = transfer_size;
spi_message_add_tail(t[2], m);
spi_sync(spi, m);
On the spi-master side the driver would need to run:
* if the spi-message (in this case the first byte) matches
the allowed command pattern:
There is no 'allowed' pattern. Any message like 1~7 bytes long can be
interpreted as read command. Unless you can tell the SPI master driver
it cannot know.
And you go the abominable route of the original patch that you compose
a SPI message in m25p80, and then decide in the SPI master driver that
you will in fact not send a SPI message and reverse-engineer what
m25p80 really meant.
On 7 August 2015 at 10:35, Vignesh R vigne...@ti.com wrote:
On 08/07/2015 01:08 PM, Michal Suchanek wrote:
Now since the description is clearer it's obvious that ti-qspi cannot
work fully mmapped as fsl-qspi does because the setup has to be done
over normal spi access and using non-m25p80 devices on the same bus is
a requirement.
The place where it is known if a transfer can use the mmap access is m25p80.c
So my suggestion is
- add a new method for spi master that gets the read opcode, dummy
length, address, address length, buffer, buffer length and performs
read from the flash memory in a hardware-specific way
- add a check in m25p80.c that the master supports this feature and if
so use it (eg check that the method is non-null)
Presumably if some new SPI controllers with similar feature are
supported in the future they can use the same inteface because you
pass on everything the m25p80 read knows.
Ok... Do you mean something like this?
I will take m25p80 as example but can be expanded for any flash.
In include/linux/mtd.h:
struct spi_mtd_config_info {
struct spi_device *spi;
u32 page_size;
u8 addr_width;
u8 erase_opcode;
u8 read_opcode;
u8 read_dummy;
u8 program_opcode;
enum read_mode flash_read;
} /* subset of struct spi_nor */
I would just pass these as separate arguments to the function but whatver.
In m25p80.c:
static int m25p80_read(struct spi_nor *nor, loff_t from,
size_t len, size_t *retlen,
u_char *buf)
{
struct spi_mtd_config_info info;
struct spi_device *spi;
if (spi-master-spi_mtd_mmap_read) {
/* Populate spi_mtd_config_info */
spi-master-spi_mtd_mmap_read(info, from, len,
retlen, buf);
}
else {
/* no mtd specific acceleration supported try normal
* SPI way of communicating with flash
* continue with current code
* set up spi_message and call spi_sync()
*/
}
}
In spi-ti-qspi.c:
Implement spi_mtd_mmap_read while holding master-bus_lock mutex.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 6 August 2015 at 18:14, Geert Uytterhoeven ge...@linux-m68k.org wrote: On Thu, Aug 6, 2015 at 3:51 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote: On the whole following are my requirements: 1. to be able to communicate with non -flash SPI devices via config port ( this functionality is supported by current driver, I dont want to break it). Or pump any spi_message on to SPI bus directly. 2. take advantage of memory mapped port in order to increase read throughput( and use dma in future) when the slave is a m25p80 type flash. 3. handle m25p80 as well as other slave on multiple chipselects. I just need to know whether the user that requested the transfer is m25p80 driver. If yes, ti-qspi driver can take advantage of memory mapped interface, else just use config port to access SPI bus directly. The problem with this approach is that it's an abomination. It's adding a SPI-user specific hack which is detected by a specific driver. That's really not sane - what happens when we have lots of these kinds of I'm an X SPI-user with drivers detecting that? It's not maintainable in the long term. Yes, your requirements _today_ seem simple and easy, but you're only thinking about today, not tomorrow when you've moved on and someone else has to maintain the mess left behind (or delete it from mainline because they're sick of dealing with a hack.) The spi_message that is received in transfer_one_message() is too generic to imply the slave device that is on the other side of the wire. IMO, the read command does not imply that the slave is m25p80 flash (besides the read opcodes vary across vendors of m25p80 and across modes). I can see both sides of the argument. Mark is saying: if the SPI driver detects that the message to be transmitted is a read command followed by the appropriate number of dummy bytes, and then the data being read _and_ it's using quad-mode access, and the hardware generates _exactly_ that in hardware using the memory mapped mode, there is no reason _not_ to use the hardware to achieve that SPI transaction. The bus activity will be identical to what happens when the SPI controller is used manually to achieve that bus sequence. You're saying: but the documentation says you can't use it for anything except m25p80. If you look at 24.5.4.1.2, it tells you what the SFI generates on the bus, which is: 1. CS active 2. Read command byte sent 3. 1-4 address bytes sent 4. 0-3 dummy bytes sent 5. data bytes read from bus 6. CS inactive So, Mark's point is if we can detect a transaction which fits _that_ bus activity, there's no reason not to use this acceleration for the transaction. What you're failing to counter with is: we don't have enough information in the SPI driver to know how many dummy bytes there are between the address bytes and the data read from the bus. Irrespective of the dummy bytes. What if the spi device is not a FLASH ROM, but some other device, which receives a data packet that accidentally looks like an m25p80 READ command? Presumably the driver would interpret some random part of the message as address and map the reply in your address space at that address from the flash mmap base. If you happen to overflow the flash memory mmap space the behaviour will probably not be well defined. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] MMC clock gating broken on a20
On 12 August 2015 at 13:55, Olliver Schinagl oliver+l...@schinagl.nl wrote: Actually, I've reverted hans's mmc: sunxi: Don't start commands while the card is busy and that makes it disapear as well. So it looks like that patch triggers the aggressiveness more? It probably inserts delays which trigger the mmc power saving. Still the delay should not be needed on the mmc interface which is connected to the card slot, should it? Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-sunxi] [PATCH 3/3] mmc: sunxi: use controller automatic clock gating.
On 12 August 2015 at 14:35, Hans de Goede hdego...@redhat.com wrote: Hi, On 12-08-15 14:23, Michal Suchanek wrote: When core does not set the MMC_QUIRK_BROKEN_CLK_GATING flag enable automatic hardware controlled clock gating on the mmc interface. Signed-off-by: Michal Suchanek hramr...@gmail.com In general this looks good, but I wonder how intensively this has been tested ? Unlike the wrong timeout it does not cause the boards to crash frequently ;-) It would be nice if somebody actively using some board atm tested this. Also given the long latencies when using manual clock on/off support, have you done any testing to check what I suspect the long latency is due to the controller flushing data to the card or something like that since it happens only under load. sort of latencies this adds, e.g. Both with and without the patch, dump all the filesystem caches: echo 3 /proc/sys/vm/drop_caches And then do: time cat /some/small/file/on/the/sdcard Do this at least 3 time both with and without the patch, and see if there are any noticable differences ? Yes, it would be good idea to test. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
