Re: Staging status of speakup
On Sat, 16 Mar 2019 10:35:43 +0100 Samuel Thibault wrote: > Chris Brannon, le ven. 15 mars 2019 18:19:39 -0700, a ecrit: > > Okash Khawaja writes: > > > Finally there is an issue where text in output buffer sometimes gets > > > garbled on SMP systems, but we can continue working on it after the > > > driver is moved out of staging, if that's okay. Basically we need a > > > reproducer of this issue. > > > > What kind of reproducer do you need here? It's straightforward to > > reproduce in casual use, at least with a software synthesizer. > > The problem is that neither Okash nor I are even casual users of > speakup, so we need a walk-through of the kind of operation that > produces the issue. It does not have to be reproducible each time it is > done. Perhaps (I really don't know what that bug is about actually) it > is a matter of putting text in the selection buffer, and try to paste it > 100 times, and once every 10 times it will be garbled, for instance. paste_selection still says /* Insert the contents of the selection buffer into the * queue of the tty associated with the current console. * Invoked by ioctl(). * * Locking: called without locks. Calls the ldisc wrongly with * unsafe methods, */ from which I deduce that with everyone using X nobody ever bothered to fix it. So before you look too hard at the speakup code you might want to review the interaction with selection.c too. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/25] Change tty_port(standard)_install's return type
On Tue, 4 Sep 2018 11:44:26 +0900 Jaejoong Kim wrote: > Many drivers with tty use the tty_stand_install(). But, there is no > need to handle the error, since it always returns 0. And what happens if another change means it can fail again. It's just a property of the current implementation that it can't. It used to fail. This seems to be a ton of unneccessary churn that will end up just having to be reversed again some day in the future. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] media: staging: atomisp: Fix an error handling path in 'lm3554_probe()'
On Fri, 2018-05-11 at 17:09 +0200, Julia Lawall wrote: > > On Fri, 11 May 2018, Christophe JAILLET wrote: > > > The use of 'fail1' and 'fail2' is not correct. Reorder these calls > > to > > branch at the right place of the error handling path. > > Maybe it would be good to improve the names at the same time? Its scheduled for deletion - please don't bother. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/3] media: atomisp: delete zero-valued struct members.
> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe_public.h
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe_public.h
> @@ -152,14 +152,6 @@ struct ia_css_pipe_config {
> };
>
Thani you that's a really good cleanup.
Alan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] media: staging: atomisp: fix for sparse "using plain integer as NULL pointer" warnings.
> There are 35 defaults defined by macros like this, most of them much > more complicated that IA_CSS_DEFAULT_ISP_MEM_PARAMS, and a few members > are initialized to non-zero values. My plan, therefore, is to convert > everything to use designated initializers, and then start removing the > zeroes afterwards. Where they are only used once in the tree it might be even cleaner to just do static struct foo = FOO_DEFAULT_BAR; foo.x = 12; foo.bar = &foo; etc in the code. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/3] media: staging: atomisp: fixed some checkpatch integer type warnings.
On Mon, 27 Nov 2017 12:44:50 + Jeremy Sowden wrote: > Changed the types of some arrays from int16_t to s16W Which are the same type, except int16_t is the standard form. No point. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] [media] staging: atomisp: convert timestamps to ktime_t
On Mon, 27 Nov 2017 16:21:41 +0100 Arnd Bergmann wrote: > timespec overflows in 2038 on 32-bit architectures, and the > getnstimeofday() suffers from possible time jumps, so the > timestamps here are better done using ktime_get(), which has > neither of those problems. > > In case of ov2680, we don't seem to use the timestamp at > all, so I just remove it. > > Signed-off-by: Arnd Bergmann Reviewed-by: Alan Cox ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 23/30] [media] atomisp: deprecate pci_get_bus_and_slot()
On Wed, 22 Nov 2017 12:20:47 + Alan Cox wrote: > On Wed, 2017-11-22 at 00:31 -0500, Sinan Kaya wrote: > > pci_get_bus_and_slot() is restrictive such that it assumes domain=0 > > as > > where a PCI device is present. This restricts the device drivers to > > be > > reused for other domain numbers. > > The ISP v2 will always been in domain 0. For the ISP it doesn't matter - if it cleans up general assumptions then you have my ACK for it. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 23/30] [media] atomisp: deprecate pci_get_bus_and_slot()
On Wed, 2017-11-22 at 00:31 -0500, Sinan Kaya wrote: > pci_get_bus_and_slot() is restrictive such that it assumes domain=0 > as > where a PCI device is present. This restricts the device drivers to > be > reused for other domain numbers. The ISP v2 will always been in domain 0. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: atomisp: add a driver for ov5648 camera sensor
> Would it make sense to first get the other drivers to upstream and > then see what's the status of atomisp? Agreed > the board specific information from firmware is conveyed to the > sensor drivers will change to what the rest of the sensor drivers are > using. I think a most straightforward way would be to amend the ACPI > tables to include the necessary information. I don't see that happening. The firmware they have today is the firmware they will always have, and for any new devices we manage to get going is probably going to end up entirely hardcoded. > For this reason I'm tempted to postpone applying this patch at least > until the other drivers are available. Yes - unless someone has a different controller and that sensor on a board so can test it that way ? Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] staging: atomisp: activate ATOMISP2401 support
On Mon, 11 Sep 2017 20:49:27 +0200 Vincent Hervieux wrote: > Currently atomisp module supports Intel's Baytrail SoC and contains > some compilation switches to support Intel's Cherrytrail SoC instead. > The patchset aims to : > - 1/2: activate ATOMISP2400 or ATOMISP2401 from the menu. > - 2/2: fix compilation errors for ATOMISP2401. > I'm not so confident with patch 2/2, as it is only working around the non > declared functions by using the 2400 path. As I couln't find any > declaration/definition for the ISP2401 missing functions...So any help would > be appreciated. > Also patch 2/2 doesn't correct any cosmetic changes reported by checkpatch.pl > as explained in TODO step 6. Please don't. Right now we know what work is to be done and tested. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: atomisp: use kvmalloc/kvzalloc
On Mon, 2017-08-07 at 21:44 +0800, Geliang Tang wrote: > Use kvmalloc()/kvzalloc() instead of atomisp_kernel_malloc() > /atomisp_kernel_zalloc(). > > Signed-off-by: Geliang Tang Definitely better now we have kvmalloc/kvzalloc. Thanks Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
> The patch is untested but I can work on this if that fits in with the > plans for tty. I think this is the right direction. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
> Sure. I can fix the tty->count mismatch based on Alan's suggestion. However I > don't understand why the exclusivity flag should belong to tty_port and not > tty_struct. It will be good to know why. We are trying to move all the flags that we can and structs into the tty_port, except any that are used internally within the struct tty level code. The main reason for this is to make the object lifetimes and locking simpler - because the tty_port lasts for the time the hardware is present. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: speakup: safely close tty
> spk_ttyio_initialise_ldisc is called separately for each module (e.g. > speakup_apollo, speakup_ltlk etc) when it is loaded. spk_ttyio_release > is also called separately for each module when it is unloaded. The ldisc > stays around until the last of the modules is unloaded. What guarantees that someone hasn't decided to set the ldisc on unrelated hardware to speakup (eg on a pty/tty pair). > > > > > I'd also btw strongly recommend putting the ldisc and the speakup tty > > driver as different modules. > Sure, that makes sense. I will do that following these patches. If the ldisc is just unregistered when the module implementing it is unloaded then the ref counts on the ldisc module should do everything needed if the above isn't correctly handled, and if it is will still be cleaner. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
> When opening from kernel, we don't use file pointer. The count mismatch > is between tty->count and #fd's. So opening from kernel leads to #fd's > being less than tty->count. I thought this difference is relevant to > user-space opening of tty, and not to kernel opening of tty. Can you > suggest how to address this mismatch? Your kernel reference is the same as having a file open reference so I think this actually needs addressing in the maths. In other words count the number of kernel references and also add that into the test for check_tty_count (kernel references + #fds == count). I'd really like to keep this right because that check has a long history of catching really nasty race conditions in the tty code. The open/close/hangup code is really fragile so worth the debugability. > Ah may be I didn't notice the active bit. Is it one of the #defines in > tty.h? Can usage count and active bit be used to differentiate between > whether the tty was opened by kernel or user? It only tells you whether the port is currently active for some purpose, not which. If you still want to implement exclusivity between kernel and user then it needs another flag, but I think that flag should be in port->flags as it is a property of the physical interface. (Take a look at tty_port_open for example) Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: speakup: safely close tty
On Fri, 7 Jul 2017 20:13:01 +0100 Okash Khawaja wrote: > Speakup opens tty using tty_open_by_driver. When closing, it calls > tty_ldisc_release but doesn't close and remove the tty itself. As a > result, that tty cannot then be opened from user space. This patch calls > tty_release_struct which ensures that tty is safely removed and freed > up. It also calls tty_ldisc_release, so speakup doesn't need to call it. > > This patch also unregisters N_SPEAKUP. It is registered when a speakup > module is loaded. What happens if after you register it someone assigns that ldisc to another tty as well ? You should register the ldisc when the relevant module is initialized and release it only when the module is unloaded. That way the module ref counts will handle cases where someone uses the ldisc with something else. I'd also btw strongly recommend putting the ldisc and the speakup tty driver as different modules. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
On Sun, 09 Jul 2017 12:41:53 +0100 Okash Khawaja wrote: > On Sat, Jul 08, 2017 at 10:38:03AM +0200, Greg Kroah-Hartman wrote: > > Overall, the idea looks sane to me. Keeping userspace from opening a > > tty that the kernel has opened internally makes sense, hopefully > > userspace doesn't get too confused when that happens. I don't think we > > normally return -EBUSY from an open call, have you seen what happens > > with apps when you do this (like minicom?) > > > I tested this wil minincom, picocom and commands like "echo foo > > /dev/ttyS0". They all correctly report "Device or resource busy". > > I have addressed all the comments you made. I have also split the patch > into three. Following is summary of each. If the tty counts are being misreported then it would be better to fix the code to actually manage the counts properly. The core tty code is telling you that the tty is not in a valid state. While this is of itself a good API to have, the underlying reference miscounting ought IMHO to be fixed as well. Also you don't need a new TTY_KOPENED flag as far as I can see. All tty's have a usage count and active bit flags already. Use those. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: atomisp: replace kmalloc & memcpy with kmemdup
On Fri, 2017-07-07 at 17:20 +0530, Hari Prasath wrote: > kmemdup can be used to replace kmalloc followed by a memcpy.This was > pointed out by the coccinelle tool. And kstrdup could do the job even better I think ? Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: atomisp: gc0310: constify acpi_device_id.
On Thu, 2017-07-06 at 21:50 +0530, Arvind Yadav wrote:
> acpi_device_id are not supposed to change at runtime. All functions
> working with acpi_device_id provided by work with
> const acpi_device_id. So mark the non-const structs as const.
>
> File size before:
> text data bss dec hex
> filename
> 10297 1888 0 12185 2f99
> drivers/staging/media/atomisp/i2c/gc0310.o
>
> File size After adding 'const':
> text data bss dec hex
> filename
> 10361 1824 0 12185 2f99
> drivers/staging/media/atomisp/i2c/gc0310.o
>
> Signed-off-by: Arvind Yadav
> ---
> drivers/staging/media/atomisp/i2c/gc0310.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/gc0310.c
> b/drivers/staging/media/atomisp/i2c/gc0310.c
> index 1ec616a..c8162bb 100644
> --- a/drivers/staging/media/atomisp/i2c/gc0310.c
> +++ b/drivers/staging/media/atomisp/i2c/gc0310.c
> @@ -1453,7 +1453,7 @@ static int gc0310_probe(struct i2c_client
> *client,
> return ret;
> }
>
> -static struct acpi_device_id gc0310_acpi_match[] = {
> +static const struct acpi_device_id gc0310_acpi_match[] = {
> {"XXGC0310"},
> {},
> };
(All four)
Acked-by: Alan Cox
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Firmware for staging atomisp driver
> I'm asking because that is hard to believe given e.g. the recursion bug > I've just fixed. It was kind of working yes (with libxcam and a simple test tool). The recursion one was my fault. I didn't mean that one to go upstream as I was still debugging it. The older code handled it right until I moved the test. > Can someone provide step-by-step instructions for how to get this > driver working? Need to restore my backup to get the set up I had (I broke my memopad somewhat with one of my tests and it ate it's disk). But basically I was using https://github.com/01org/libxcam/wiki/Tests Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Firmware for staging atomisp driver
On Sun, 2017-05-28 at 14:30 +0200, Hans de Goede wrote: > I started with an Asus T100TA after fixing 2 oopses in the sensor > driver there I found out that the BIOS does not allow to put the > ISP in PCI mode and that there is no code to drive it in ACPI > enumerated mode. In ACPI mode it's enumerated as part of the GPU but beyond that I don't know how it works and haven't been able to find out. I am guessing at this point that the extra node in the acpidump under the GPU gives the base address, but no idea where the IRQ comes from. Power management on the BYT devices via PCI is broken anyway so that bit shouldn't matter. > So I moved to a generic Insyde T701 tablet which does allow > this. After fixing some more sensor driver issues there I was > ready to actually load the atomisp driver, but I could not > find the exact firmware required, I did find a version which > is close: "irci_stable_candrpv_0415_20150423_1753" > and tried that but that causes the atomisp driver to explode > in a backtrace which contains atomisp_load_firmware() so that > one seems no good. The firmware has to exactly match. If you look in the atomisp code you'll see that there are offsets which basically appear to be memory addresses in the firmware as built that time. > Can someone help me to get the right firmware ? Yep. > The TODO says: "can also be extracted from the upgrade kit" > about the firmware files, but it is not clear to me what / > where the "upgrade kit" is. The Android upgrade kit for your device. So your platforms equivalent of https://www.asus.com/Tablets/ASUS_MeMO_Pad_7_ME176C/HelpDesk_Download/ > More in general it would be a good idea if someone inside Intel > would try to get permission to add the firmware to linux-firmware. I spent 6 months trying, but even though you can go to your product vendors website and download it I've not been able to 8(. > > Anyways I will send out the patches I've currently, once I've > the right firmware I will continue working on this. Really appreciate the work you are doing on this. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: atomisp: lm3554: fix sparse warnings(was not declared. Should it be static?)
On Mon, 29 May 2017 02:06:41 +0800 Chen Guanqiao wrote: > Fix "symbol 'xxx' was not declared. Should it be static?" sparse warnings. > > Signed-off-by: Chen Guanqiao > --- Reviewed-by: Alan Cox ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
> > Signed-off-by: Haiyang Zhang > > --- > > According to Stephen Hemminger , there are > additional programs, like X.org, DPDK, are also using 16-bit only > PCI domain numbers. So, I'm submitting this patch for re-consideration. The correct way to handle this is to send the needed patches to DPDK and to X.org both of whom will I am sure be delighted to get it fixed in their codebase. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: media: atomisp queued up patches
On Thu, 2017-05-18 at 11:10 -0300, Mauro Carvalho Chehab wrote: > Em Thu, 18 May 2017 15:50:09 +0200 > Greg Kroah-Hartman escreveu: > > > > > Hi Mauro, > > > > Here's the set of accumulated atomisp staging patches that I had in > > my > > to-review mailbox. After this, my queue is empty, the driver is > > all > > yours! > > Thanks! > > Alan, please let me know if you prefer if I don't apply any of > such patches, otherwise I should be merging them tomorrow ;) I will assume you've merged them and resync the internal patch queue I have here to that. At the moment I'm still slowly trying to unthread some of the fascinating layers of indirection without actually breaking anything. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: media: fix missing blank line coding style issue in atomisp_tpg.c
On Wed, 2017-05-17 at 21:48 -0400, Manny Vindiola wrote:
> This is a patch to the atomisp_tpg.c file that fixes up a missing
> blank line warning found by the checkpatch.pl tool
>
> Signed-off-by: Manny Vindiola
> ---
> drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
> index 996d1bd..48b9604 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_tpg.c
> @@ -56,6 +56,7 @@ static int tpg_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_format *format)
> {
> struct v4l2_mbus_framefmt *fmt = &format->format;
> +
> if (format->pad)
> return -EINVAL;
> /* only raw8 grbg is supported by TPG */
The TODO fille for this driver specifically says not to send formatting
patches at this point.
There is no point making trivial spacing changes in code that needs
lots of real work. It's like polishing your car when the doors have
fallen off.
Alan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 1/1] staging: speakup: flush tty buffers and ensure hardware flow control
On Fri, 12 May 2017 20:35:18 +0100
Okash Khawaja wrote:
> On Thu, May 11, 2017 at 02:33:14PM +0100, Alan Cox wrote:
> > On Thu, 11 May 2017 09:29:14 +0100
> > Okash Khawaja wrote:
> >
> > > Hi Alan,
> > >
> > > On Wed, May 10, 2017 at 08:41:51PM +0100, Alan Cox wrote:
> > > > > + if (!(tmp_termios.c_cflag & CRTSCTS)) {
> > > > > + tmp_termios.c_cflag |= CRTSCTS;
> > > > > + ret = tty_set_termios(tty, &tmp_termios);
> > > > > + if (ret)
> > > > > + pr_warn("speakup: Failed to set hardware flow
> > > > > control\n");
> > > >
> > > > You should check the tty c_cflag after the call rather than rely on an
> > > > error code. Strictly speaking tty_set_termios should error if no tty
> > > > bits
> > > > are changed by the request but it never has on Linux. Instead check the
> > > > tty gave you the result you wanted.
> > > Thanks. I will replace the check for return value with check for c_cflag.
> > >
> > > May be we should fix this in tty_set_termios?
> >
> > Possibly. It however changes the external kernel ABI. It's also not a
> > simple memcmp because any undefined bits must be ignored.
> >
> > Make a patch, try it and see what breaks ? If nothing breaks then yes it
> > makes sense IMHO too.
>
> Right, thanks for the heads up. I'll see if I get round to doing it. For
> external kernel ABI, will we need to check libc implementations of
> termios functions? Will that be sufficient?
I think the only way you find out is to try it and run a distro like
that. If it seems ok submit the patch to linux-next with a clear note it
potentially changes the ABI so if people find bugs it can be reverted.
Alan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 1/1] staging: speakup: flush tty buffers and ensure hardware flow control
On Thu, 11 May 2017 09:29:14 +0100
Okash Khawaja wrote:
> Hi Alan,
>
> On Wed, May 10, 2017 at 08:41:51PM +0100, Alan Cox wrote:
> > > + if (!(tmp_termios.c_cflag & CRTSCTS)) {
> > > + tmp_termios.c_cflag |= CRTSCTS;
> > > + ret = tty_set_termios(tty, &tmp_termios);
> > > + if (ret)
> > > + pr_warn("speakup: Failed to set hardware flow
> > > control\n");
> >
> > You should check the tty c_cflag after the call rather than rely on an
> > error code. Strictly speaking tty_set_termios should error if no tty bits
> > are changed by the request but it never has on Linux. Instead check the
> > tty gave you the result you wanted.
> Thanks. I will replace the check for return value with check for c_cflag.
>
> May be we should fix this in tty_set_termios?
Possibly. It however changes the external kernel ABI. It's also not a
simple memcmp because any undefined bits must be ignored.
Make a patch, try it and see what breaks ? If nothing breaks then yes it
makes sense IMHO too.
Alan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 1/1] staging: speakup: flush tty buffers and ensure hardware flow control
> + if (!(tmp_termios.c_cflag & CRTSCTS)) {
> + tmp_termios.c_cflag |= CRTSCTS;
> + ret = tty_set_termios(tty, &tmp_termios);
> + if (ret)
> + pr_warn("speakup: Failed to set hardware flow
> control\n");
You should check the tty c_cflag after the call rather than rely on an
error code. Strictly speaking tty_set_termios should error if no tty bits
are changed by the request but it never has on Linux. Instead check the
tty gave you the result you wanted.
Alan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/media: make atomisp vlv2_plat_clock explicitly non-modular
> I'm pretty sure we want this code to be built as a module, so maybe a > Kconfig change would resolve the issue instead? > > Alan, any thoughts? It's a tiny chunk of platform helper code. It probably ultimately belongs in arch/x86 somewhere or folded into the driver. At the moment it won't build modular. I'm fine with the change, it strips out more pointless code so helps see what tiny bits of code in there are actually used for anything real. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: atomisp: remove enable_isp_irq function and add disable_isp_irq
On Fri, 2017-04-07 at 14:56 +0900, Daeseok Youn wrote: > Enable/Disable ISP irq is switched with "enable" parameter of > enable_isp_irq(). It would be better splited to two such as > enable_isp_irq()/disable_isp_irq(). > > But the enable_isp_irq() is no use in atomisp_cmd.c file. > So remove the enable_isp_irq() function and add > disable_isp_irq function only. All 3 added to my tree - thanks Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: atomisp: simplify the if condition in atomisp_freq_scaling()
On Thu, 2017-03-30 at 15:24 +0900, Daeseok Youn wrote: > The condition line in if-statement is needed to be shorthen to > improve readability. > > Signed-off-by: Daeseok Youn > --- How about a define for ATOMISP_IS_CHT(isp) instead - as we will need these tests in other places where there are ISP2400/ISP2401 ifdefs ? Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: media: atomisp: Fix style. remove space before ',' and convert to tabs.
On Wed, 2017-03-29 at 09:57 -0700, Daniel Cashman wrote: > From: Dan Cashman > > Signed-off-by: Dan Cashman As the TODO asks - please no whitespace cleanups yet. They make it harder to keep other cleanups that fix (or mostly remove) code applying. Nothing wrong with the patch otherwise - but it should also have something in the commit message. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: atomisp: remove ifdef around HMM_BO_ION
> > 2 -- > > 1 file changed, 2 deletions(-) > > Ugh, Alan, what's going on here, I thought you fixed this? I sent you a patch that removed the arrays entirely and turned it into a single string as well as removing the define. Not quite sure what happened but I've resynched to -next and I'll send you it with the next batch of patches. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: atomisp: fix build error
On Thu, 2017-03-23 at 21:12 +0800, Geliang Tang wrote: > Fix the following build error: > > CC drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.o > drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c:52:2: > error: excess elements in array initializer [-Werror] > "i", /* ion */ > ^~~ NAK I've sent a patch to sort this out properly we shouldn't be using string arrays for single char values to start with... Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [bug report] staging/atomisp: Add support for the Intel IPU v2
> 457 if (ret) > 458 return ret; > ^^ > We're returning directly with the lock held. We should either unlock > before returning or do a goto out but I'm not sure which. unlock and return (although clearly this path never happens in the real world since that bug is in lots of product 8)). Added to my queue. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: media: atomisp: fill properly hmm_bo_type_strings when ION is disabled
On Wed, 2017-03-15 at 14:09 -0400, Jérémy Lefaure wrote: > When CONFIG_ION is disabled, HMM_BO_LAST is 3. In this case, "i" > should > not be added in hmm_bo_type_strings. No the goal is to remove ifdefs - that's not the way to go with driver clean up. Instead the array size should always cover the four strings. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: media: atomisp: fix build when PM is disabled
> + if (IS_ENABLED(CONFI_PM)) {
I think not.
Please actually test build both ways at the very least.
Alan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset
On Mon, 2017-03-13 at 19:54 +0900, Daeseok Youn wrote: > If the atomisp_kernel_zalloc() has "true" as a second parameter, it > tries to allocate zeroing memory from kmalloc(vmalloc) and memset. > But using kzalloc is rather than kmalloc followed by memset with 0. > (vzalloc is for same reason with kzalloc) This is true but please don't apply this. There are about five other layers of indirection for memory allocators that want removing first so that the driver just uses the correct kmalloc/kzalloc/kv* functions in the right places. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: atomisp: potential underflow in atomisp_get_metadata_by_type()
On Mon, 2017-03-13 at 15:34 +0300, Dan Carpenter wrote: > md_type is an enum. On my tests, GCC treats it as unsigned but > according to the C standard it's an implementation dependant thing so > we > should check for negatives. Can do but the kernel assumes GNU C everywhere else. Acked-by: Alan Cox Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [staging:staging-testing 209/209] drivers/staging/media/atomisp/i2c/imx/otp_brcc064_e2prom.c:72:3-8: WARNING: invalid free of devm_ allocated data (fwd)
On Wed, 2017-03-01 at 11:25 +0100, Julia Lawall wrote: > In both of the cited files (only one is shown), buffer is allocated > using > devm_kzalloc, so the kfree is not appropriate. > Thanks I'll get these fixed up. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: goldfish: Add DMA support using dma_alloc_coherent
> Yes I will free it using dma_free_coherent. Why should devm_kzalloc > be > replaced with dma_alloc_coherent ? I was trying to replace _pa() Why keep allocating and freeing a buffer rather than having a single buffer allocated once (as it is in the old driver). Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: goldfish: Add DMA support using dma_alloc_coherent
> unnecessarily used when DMA can do the job. Coherent mapping is > allocated > by means of dma_alloc_coherent so that the device and CPU are in > sync. It's also not freed 8) As far as I can see you can replace the devm_kzalloc of cmd->params with a dma_alloc_coherent, and ensure you free it when the device is destroyed, rather than allocating a new buffer each command. You also want dma_handle to be a u64 - dma_handle_t might be 32 or 64 bit and due to a historical (and IMHO dim) bit of design C doesn't define right or left shifting by >= the size of the type as "0" but as "undefined". Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: i2o: Remove unwanted semicolon
> > Possibly but that ought to go via staging and really is one for the SCSI > > folks to call. The dpt_i2o was a bit more common than i2o proper. > > But if the staging i2o core is removed, doesn't that mean that this > driver will stop working? It uses code in uapi i2o.h, which I'm > guessing is implemented in the staging i2o core. They are separate drivers. dpt_i2o intentionally shares some API bits but they stand alone. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: i2o: Remove unwanted semicolon
On Fri, 2015-05-01 at 09:41 +0200, gre...@linuxfoundation.org wrote: > On Thu, Apr 30, 2015 at 11:25:48PM +0100, One Thousand Gnomes wrote: > > On Thu, 30 Apr 2015 16:14:06 +0200 > > "gre...@linuxfoundation.org" wrote: > > > > > On Thu, Apr 23, 2015 at 04:09:28PM +0100, Alan Cox wrote: > > > > On Thu, 2015-04-23 at 13:43 +, Gujulan Elango, Hari Prasath (H.) > > > > wrote: > > > > > This patch removes unwanted semicolon around close braces of code > > > > > blocks > > > > > > > > > > > > The i2o driver moved into staging ready to be deleted unless someone > > > > steps up with hardware willing to maintain it (which is rather > > > > unlikely). > > > > > > I think it's now time to delete these, want me to do that for 4.2? I > > > can queue that up in my tree now, so that we don't see any more cleanup > > > patches being made for them? > > > > Yeah I think it can go > > I was about to delete it, but what about drivers/scsi/dpt/dpti_i2o.* ? > Should that be removed as well? Possibly but that ought to go via staging and really is one for the SCSI folks to call. The dpt_i2o was a bit more common than i2o proper. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: i2o: Remove unwanted semicolon
On Thu, 2015-04-23 at 13:43 +, Gujulan Elango, Hari Prasath (H.) wrote: > This patch removes unwanted semicolon around close braces of code blocks The i2o driver moved into staging ready to be deleted unless someone steps up with hardware willing to maintain it (which is rather unlikely). Nothing wrong with removing all those semi-colons or the submission, it's just you are patching something going away! Thanks for the patch though - and better luck next time 8) Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: i2o: fixed various code style issues in i2o_block.c
On Sat, 2015-04-18 at 22:12 +0200, Yorick Rommers wrote: > From: Yorick > > This is a patch that fixes errors regarding whitespaces and split strings. Wouldn't worry. The i2o layer is in staging because it is about to be dropped from the kernel unless someone steps up to maintain it. Given its unlikely anyone even has hardware I doubt that will happen. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4] Fix pointer cast for 32 bits arch
On Fri, 2015-04-17 at 16:59 +0300, Dan Carpenter wrote: > On Fri, Apr 17, 2015 at 02:31:49PM +0100, Alan Cox wrote: > > On Fri, 2015-04-17 at 11:20 +0300, Dan Carpenter wrote: > > > Actually, my patch seems like a good idea to me but it's one of those > > > things that someone should probably test. Unless someone can test > > > goldfish on a 32 bit system with 64 bit dma addresses > > > > No such "system" exists. > > I don't understand. We definitely can have 64bit dma addresses on > x86_32. Yes but no actual Goldfish environment is built that way ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4] Fix pointer cast for 32 bits arch
On Fri, 2015-04-17 at 11:20 +0300, Dan Carpenter wrote: > Actually, my patch seems like a good idea to me but it's one of those > things that someone should probably test. Unless someone can test > goldfish on a 32 bit system with 64 bit dma addresses No such "system" exists. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4] Fix pointer cast for 32 bits arch
On Thu, 2015-04-16 at 20:01 +0300, Dan Carpenter wrote:
> On Thu, Apr 16, 2015 at 06:14:55PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Apr 16, 2015 at 3:39 PM, Peter Senna Tschudin
> > wrote:
> > > --- a/drivers/staging/goldfish/goldfish_audio.c
> > > +++ b/drivers/staging/goldfish/goldfish_audio.c
> > > @@ -63,7 +63,7 @@ struct goldfish_audio {
> > > #define AUDIO_READ(data, addr) (readl(data->reg_base + addr))
> > > #define AUDIO_WRITE(data, addr, x) (writel(x, data->reg_base + addr))
> > > #define AUDIO_WRITE64(data, addr, addr2, x)\
> > > - (gf_write64((u64)(x), data->reg_base + addr,
> > > data->reg_base+addr2))
> > > + (gf_write_ptr((void *)(x), data->reg_base + addr,
> > > data->reg_base+addr2))
> >
> > This one should not be converted, as all callers pass a dma_addr_t, which
> > may
> > be 64-bit on 32-bit systems, i.e. larger than void *.
>
> Ugh... You're right.
>
> I've been avoiding asking this but I can't any longer. What is
> gf_write64() actually doing? We are writing dma addresses, user space
> pointers and kernel space pointers to this hardware?
>
> This stuff doesn't seem to make any kind of sense and I can easily
> imagine a situation where it wrote a 64 bit pointer. Then we partially
> write over it with a 32 bit userspace pointer. Then it writes somewhere
> totally unintended.
>
> This thing doesn't make any sort of sense to me.
Its a 64 on 64 or 32 on 32 virtual machine. Goldfish is used for Android
emulation for all the system level phone emulation tools. On the
emulation side it provides an interface for the emulated OS but makes no
effort to emulate it as if it was a real hardware. If you think of it as
a funky emulator interface all is good. If you think about it as
"hardware" you've got the wrong model and chunks of Goldfish make less
sense.
Alan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: goldfish_audio.c: sparse warning of incorrect type
On Sun, 2014-08-31 at 13:02 -0700, Greg Kroah-Hartman wrote: > Adding Alan Cox, as he pushed this driver upstream... It's sort of a false positive. The existing code will work fine. Arguably all of this should be using the dma_ APIs. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Anybody working on sep?
On Fri, 2014-07-25 at 21:54 -0700, Greg KH wrote: > On Fri, Jul 25, 2014 at 10:54:47PM +0100, Alan Cox wrote: > > On Fri, 2014-07-25 at 20:17 +0300, Kristina Martšenko wrote: > > > On 23/06/14 23:32, Kristina Martšenko wrote: > > > > Hi Mark, > > > > > > > > I'm helping Greg do a bit of cleanup in the staging tree. I noticed that > > > > nobody seems to have worked towards moving sep out of staging in over a > > > > year. Are there any plans to clean it up and move it out soon? > > > > > > No response from Mark here. > > > > > > Alan, Jayant, either of you know what the status of the sep staging > > > driver is? > > > > I believe "works last time anyone checked". I don't think we have anyone > > officially working on the early Intel phone cpus for upstream any more. > > So can we just drop it then? If no one is working on moving it out of > staging, it should go. Probably ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Anybody working on sep?
On Fri, 2014-07-25 at 20:17 +0300, Kristina Martšenko wrote: > On 23/06/14 23:32, Kristina Martšenko wrote: > > Hi Mark, > > > > I'm helping Greg do a bit of cleanup in the staging tree. I noticed that > > nobody seems to have worked towards moving sep out of staging in over a > > year. Are there any plans to clean it up and move it out soon? > > No response from Mark here. > > Alan, Jayant, either of you know what the status of the sep staging > driver is? I believe "works last time anyone checked". I don't think we have anyone officially working on the early Intel phone cpus for upstream any more. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Status of RMI4 drivers?
On Fri, 2014-07-04 at 21:48 +0300, Kristina Martšenko wrote: > Hi, > > I'm going over some "older" drivers in the staging tree and wanted to > ask about cptm1217 and ste_rmi4. They've been in staging for three and a > half years now, waiting for the upstream Synaptics RMI4 drivers. From > what I understand, the RMI4 development is happening in the > synaptics-rmi4 branch of Dmitry's git tree. Does anyone have any idea > when the RMI4 code might be "ready" and get merged properly? How is that > going? I'd be quite happy to move cptm1217 out of staging, however its now been there so many years waiting the rumoured RMI4 driver that I don't have hardware to do any testing on it, so it would need to be a straight move. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: goldfish: fix direct copy_to_user() from __iomem
On Sun, 2014-06-22 at 03:29 -0400, James A Shackleford wrote: > This patch allocates a few pages and performs an ioread8_rep() from the bus > address, which are then copied to userspace. This fixes the sparse warning: > > drivers/staging/goldfish/goldfish_audio.c:136:43: warning: incorrect type in > argument 2 (different address spaces) > drivers/staging/goldfish/goldfish_audio.c:136:43:expected void const *from > drivers/staging/goldfish/goldfish_audio.c:136:43:got char [noderef] > *read_buffer > > which was a result of performing a copy_to_user() directly from the bus > address > to the userspace, which can be unsafe across some architectures. Goldfish is a specific architecture. It shouldn't be doing direct xcopies like that - which is why the code is in staging but at the same time allocating a buffer each call and doing extra copies is not the right answer. It should be mapping the pages when they are first set up. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: et131x: let the freeing of memory more reasonable in error path
On Mon, 2014-02-24 at 16:54 -0800, Greg Kroah-Hartman wrote: > On Sat, Feb 22, 2014 at 12:33:36PM +0800, Zhao, Gang wrote: > > On Fri, 2014-02-21 at 20:35:50 +0800, Dan Carpenter wrote: > > > On Fri, Feb 21, 2014 at 08:22:21PM +0800, Zhao, Gang wrote: > > >> > If we add your patch and the reviewer does a search for fb[0] then it > > >> > is > > >> > confusing what the right thing to do is. > > >> > > >> My fault. I should remove that two lines of code in > > >> et131x_rx_dma_memory_free(), although they don't break the code. > > >> > > > > > > Think about what you are saying here for a minute. > > > > Oh, a cold makes me stupid. that two lines of code is needed > > definitively. > > > > So is additional modification needed to let this patch be accepted ? > > As I can't take this as-is, yes. Sorry I'm completely lost here, can someone explain the problem they are seeing after the changes ? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
