Re: [Outreachy kernel] [PATCH] staging: vboxvideo: Removed unnecessary parentheses
On Mon, 22 Oct 2018, Shayenne da Luz Moura wrote: > This patch fixes the checkpatch.pl check: > > vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 < > crtc->x' > vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].y2 < > crtc->y' Please use the imperative, in the subject line and in the patch. Also, try to say something other than "fixes" about what you do, which is not very descriptive. Say what you actually did and why, eg remove unneeded parentheses around the arguments of || to reduce clutter. It is useful to mention that the change was suggested by checkpatch. It is not essential to copy the whole checkpatch message, or at least not all of them. thanks, julia > > Signed-off-by: Shayenne da Luz Moura > --- > drivers/staging/vboxvideo/vbox_main.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/vboxvideo/vbox_main.c > b/drivers/staging/vboxvideo/vbox_main.c > index 429f6a453619..10a862674789 100644 > --- a/drivers/staging/vboxvideo/vbox_main.c > +++ b/drivers/staging/vboxvideo/vbox_main.c > @@ -116,10 +116,10 @@ void vbox_framebuffer_dirty_rectangles(struct > drm_framebuffer *fb, > struct vbva_cmd_hdr cmd_hdr; > unsigned int crtc_id = to_vbox_crtc(crtc)->crtc_id; > > - if ((rects[i].x1 > crtc->x + crtc->hwmode.hdisplay) || > - (rects[i].y1 > crtc->y + crtc->hwmode.vdisplay) || > - (rects[i].x2 < crtc->x) || > - (rects[i].y2 < crtc->y)) > + if (rects[i].x1 > crtc->x + crtc->hwmode.hdisplay || > + rects[i].y1 > crtc->y + crtc->hwmode.vdisplay || > + rects[i].x2 < crtc->x || > + rects[i].y2 < crtc->y) > continue; > > cmd_hdr.x = (s16)rects[i].x1; > -- > 2.19.1 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/2018103126.smnsopiu4cavq6zh%40smtp.gmail.com. > For more options, visit https://groups.google.com/d/optout. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: adt7316: Switch to the gpio descriptor interface
On Mon, Oct 22, 2018 at 10:44:22PM +0530, Nishad Kamdar wrote:
> - chip->ldac_pin = adt7316_platform_data[1];
> + chip->ldac_pin = devm_gpiod_get(dev, "ldac", GPIOD_OUT_HIGH);
> + if (IS_ERR(chip->ldac_pin)) {
> + ret = PTR_ERR(chip->ldac_pin);
> + dev_err(dev, "Failed to request ldac GPIO: %d\n",
> + ret);
This can fit on one line:
dev_err(dev, "Failed to request ldac GPIO: %d\n", ret);
regards,
dan carpenter
> + return ret;
> + }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On Mon, Oct 22, 2018 at 05:47:08PM +0300, Alexey Skidanov wrote: > > > On 10/22/18 17:32, Laura Abbott wrote: > > On 10/22/2018 07:02 AM, Alexey Skidanov wrote: > >> The heap name might be uninitialized and access might crash the > >> kernel. > >> > > > > The heap name should never be null so this seems like this is being > > fixed in the wrong place. Can you explain more how you are hitting > > this issue? > Sure. Carve out heap name is uninitialized. There is the next patch > fixing it. But to be on the safe side, I have added the check. > You keep saying uninitialized but you mean NULL. ion_carveout_heap_create() is never called so far as I can see so this isn't an issue in real life. It feels like it would be detected right away when that code was used. We should just apply your follow on patch instead. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Custom Made USB drives with your logo
Hi, I didn’t know if you had received my email from last week? Can you direct me to the person that handles your company promo items? Do you have any upcoming events, tradeshows or promotional needs? We manufacture ALL custom LOGO and branded products. The most asked about product that we make, is the custom printed USB flash drives! We can print your logo on them and load your digital images, videos and files! If you need marketing, advertising, gifts or incentives, USB flash drives are the solution! Here is what we include: -Any size memory you need: 64MB up to 128GB -We will print your logo on both sides, just ask! -Very Low Order Minimums -Need them quickly? Not a problem, we offer Rush Service NEW: We can make a custom shaped USB drive to look like your Logo or product! Email over a copy of your logo and we will create a design mock up for you at no cost! Our higher memory sizes are a really good option right now! Ask about the “Double Your Memory” upgrade promotion going on right now! Pricing is low right now, so let us know what you need and we will get you a quick quote. We will beat any competitors pricing, send us your last invoice and we will beat it! We always offer great rates for schools and nonprofits as well. Regards, Lilly Koller Logo USB Account Manager ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vboxvideo: Removed unnecessary parentheses
This patch fixes the checkpatch.pl check: vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 < crtc->x' vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].y2 < crtc->y' Signed-off-by: Shayenne da Luz Moura --- drivers/staging/vboxvideo/vbox_main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 429f6a453619..10a862674789 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -116,10 +116,10 @@ void vbox_framebuffer_dirty_rectangles(struct drm_framebuffer *fb, struct vbva_cmd_hdr cmd_hdr; unsigned int crtc_id = to_vbox_crtc(crtc)->crtc_id; - if ((rects[i].x1 > crtc->x + crtc->hwmode.hdisplay) || - (rects[i].y1 > crtc->y + crtc->hwmode.vdisplay) || - (rects[i].x2 < crtc->x) || - (rects[i].y2 < crtc->y)) + if (rects[i].x1 > crtc->x + crtc->hwmode.hdisplay || + rects[i].y1 > crtc->y + crtc->hwmode.vdisplay || + rects[i].x2 < crtc->x || + rects[i].y2 < crtc->y) continue; cmd_hdr.x = (s16)rects[i].x1; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: bcm2835-audio: interpolate audio delay
Hello Mike,
AFAIU, this patch is wrong. Please correct me, maybe I misunderstand something.
> The problem that this patch seeks to resolve is that when userland asks for
> the delay
The userspace asks not for delay, but for the pointer.
You modify the function, which is called `snd_bcm2835_pcm_pointer`. Here you are
supposed to increase `alsa_stream->pos` with the proper offset. Instead, you
imitate a delay, but in fact the delay is not increased.
So, the proper solution should be to fix the reported pointer. As a result,
userspace will recieve the correct delay, instead of these crazy 10 ms.
> FYI, there is
> a discussion of the effects of a downstream equivalent of this suggested patch
> at:
> https://github.com/raspberrypi/firmware/issues/1026#issuecomment-415746016.
Thank you for the link, it clarified for me what you try to achieve.
On 10/22/18 21:17, Mike Brady wrote:
> When the BCM2835 audio output is used, userspace sees a jitter up to 10ms
> in the audio position, aka "delay" -- the number of frames that must
> be output before a new frame would be played.
> Make this a bit nicer for userspace by interpolating the position
> using the CPU clock.
> The overhead is small -- an extra ktime_get() every time a GPU message
> is sent -- and another call and a few calculations whenever the delay
> is sought from userland.
> At 48,000 frames per second, i.e. approximately 20 microseconds per
> frame, it would take a clock inaccuracy of
> 20 microseconds in 10 milliseconds -- 2,000 parts per million --
> to result in an inaccurate estimate, whereas
> crystal- or resonator-based clocks typically have an
> inaccuracy of 10s to 100s of parts per million.
>
> Signed-off-by: Mike Brady
> ---
> Changes in v2 -- remove inappropriate addition of SNDRV_PCM_INFO_BATCH flag
>
> .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 20 +++
> .../vc04_services/bcm2835-audio/bcm2835.h | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> index e66da11af5cf..9053b996cada 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> @@ -74,6 +74,7 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream
> *alsa_stream,
> atomic_set(_stream->pos, pos);
>
> alsa_stream->period_offset += bytes;
> + alsa_stream->interpolate_start = ktime_get();
> if (alsa_stream->period_offset >= alsa_stream->period_size) {
> alsa_stream->period_offset %= alsa_stream->period_size;
> snd_pcm_period_elapsed(substream);
> @@ -243,6 +244,7 @@ static int snd_bcm2835_pcm_prepare(struct
> snd_pcm_substream *substream)
> atomic_set(_stream->pos, 0);
> alsa_stream->period_offset = 0;
> alsa_stream->draining = false;
> + alsa_stream->interpolate_start = ktime_get();
>
> return 0;
> }
> @@ -292,6 +294,24 @@ snd_bcm2835_pcm_pointer(struct snd_pcm_substream
> *substream)
> {
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct bcm2835_alsa_stream *alsa_stream = runtime->private_data;
> + ktime_t now = ktime_get();
> +
> + /* Give userspace better delay reporting by interpolating between GPU
> + * notifications, assuming audio speed is close enough to the clock
> + * used for ktime
> + */
> +
> + if ((ktime_to_ns(alsa_stream->interpolate_start)) &&
> + (ktime_compare(alsa_stream->interpolate_start, now) < 0)) {
> + u64 interval =
> + (ktime_to_ns(ktime_sub(now,
> + alsa_stream->interpolate_start)));
> + u64 frames_output_in_interval =
> + div_u64((interval * runtime->rate), 10);
> + snd_pcm_sframes_t frames_output_in_interval_sized =
> + -frames_output_in_interval;
> + runtime->delay = frames_output_in_interval_sized;
> + }
>
> return snd_pcm_indirect_playback_pointer(substream,
> _stream->pcm_indirect,
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
> index e13435d1c205..595ad584243f 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
> @@ -78,6 +78,7 @@ struct bcm2835_alsa_stream {
> unsigned int period_offset;
> unsigned int buffer_size;
> unsigned int period_size;
> + ktime_t interpolate_start;
>
> struct bcm2835_audio_instance *instance;
> int idx;
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Add carve out heap name initialization
Heap name is mundatory. Signed-off-by: Alexey Skidanov --- drivers/staging/android/ion/ion_carveout_heap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c index e129237..e89b464 100644 --- a/drivers/staging/android/ion/ion_carveout_heap.c +++ b/drivers/staging/android/ion/ion_carveout_heap.c @@ -131,6 +131,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data) gen_pool_add(carveout_heap->pool, carveout_heap->base, heap_data->size, -1); carveout_heap->heap.ops = _heap_ops; + carveout_heap->heap.name = heap_data->name; carveout_heap->heap.type = ION_HEAP_TYPE_CARVEOUT; carveout_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: iio: ad5933: Remove unnecessary blank line
This patch fixes the checkpatch.pl warning: WARNING: Blank lines aren't necessary before a close brace '}' Signed-off-by: Victor Colombo --- drivers/staging/iio/impedance-analyzer/ad5933.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c index 7a216ea90784..f9bcb8310e21 100644 --- a/drivers/staging/iio/impedance-analyzer/ad5933.c +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c @@ -267,7 +267,6 @@ static void ad5933_calc_out_ranges(struct ad5933_state *st) for (i = 0; i < 4; i++) st->range_avail[i] = normalized_3v3[i] * st->vref_mv / 3300; - } /* -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: iio: ad5933: Remove unnecessary space on casting
This patch fixes the checkpatch.pl warning:
WARNING: No space is necessary after a cast
Signed-off-by: Victor Colombo
---
drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 8a920f675b83..7a216ea90784 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -210,7 +210,7 @@ static int ad5933_set_freq(struct ad5933_state *st,
u8 d8[4];
} dat;
- freqreg = (u64) freq * (u64) (1 << 27);
+ freqreg = (u64)freq * (u64)(1 << 27);
do_div(freqreg, st->mclk_hz / 4);
switch (reg) {
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] staging: iio: ad5933: Remove unnecessary spaces
This patch fixes the checkpatch.pl warnings: WARNING: Blank lines aren't necessary before a close brace '}' WARNING: No space is necessary after a cast Victor Colombo (2): staging: iio: ad5933: Remove unnecessary space on casting staging: iio: ad5933: Remove unnecessary blank line drivers/staging/iio/impedance-analyzer/ad5933.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: Remove the mt29f_spinand driver
A new SPI NAND subsystem has been added in drivers/mtd/nand/spi/ and
Micron's MT29F devices are now supported in
drivers/mtd/nand/spi/micron.c.
Remove the old driver.
Signed-off-by: Boris Brezillon
---
Hello,
If anything is missing in drivers/mtd/nand/spi/micron.c to properly
support the devices supported by the mt29f_spinand driver, please let
me know.
I might accept to delay removal of this driver if I have some guarantees
that existing users will actually switch to the new driver at some
point.
Regards,
Boris
---
drivers/staging/Kconfig | 2 -
drivers/staging/Makefile | 1 -
drivers/staging/mt29f_spinand/Kconfig | 16 -
drivers/staging/mt29f_spinand/Makefile| 1 -
drivers/staging/mt29f_spinand/TODO| 13 -
drivers/staging/mt29f_spinand/mt29f_spinand.c | 980 --
drivers/staging/mt29f_spinand/mt29f_spinand.h | 106 --
7 files changed, 1119 deletions(-)
delete mode 100644 drivers/staging/mt29f_spinand/Kconfig
delete mode 100644 drivers/staging/mt29f_spinand/Makefile
delete mode 100644 drivers/staging/mt29f_spinand/TODO
delete mode 100644 drivers/staging/mt29f_spinand/mt29f_spinand.c
delete mode 100644 drivers/staging/mt29f_spinand/mt29f_spinand.h
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 1abf76be2aa8..92e3cb82c5da 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -78,8 +78,6 @@ source "drivers/staging/goldfish/Kconfig"
source "drivers/staging/netlogic/Kconfig"
-source "drivers/staging/mt29f_spinand/Kconfig"
-
source "drivers/staging/dgnc/Kconfig"
source "drivers/staging/gs_fpgaboot/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index ab0cbe8815b1..ec54f8f1cb5d 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -30,7 +30,6 @@ obj-$(CONFIG_LTE_GDM724X) += gdm724x/
obj-$(CONFIG_FIREWIRE_SERIAL) += fwserial/
obj-$(CONFIG_GOLDFISH) += goldfish/
obj-$(CONFIG_DGNC) += dgnc/
-obj-$(CONFIG_MTD_SPINAND_MT29F)+= mt29f_spinand/
obj-$(CONFIG_GS_FPGABOOT) += gs_fpgaboot/
obj-$(CONFIG_UNISYSSPAR) += unisys/
obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD) += clocking-wizard/
diff --git a/drivers/staging/mt29f_spinand/Kconfig
b/drivers/staging/mt29f_spinand/Kconfig
deleted file mode 100644
index f3f9cb3b5c35..
--- a/drivers/staging/mt29f_spinand/Kconfig
+++ /dev/null
@@ -1,16 +0,0 @@
-config MTD_SPINAND_MT29F
- tristate "SPINAND Device Support for Micron"
- depends on MTD_NAND && SPI
- help
- This enables support for accessing Micron SPI NAND flash
- devices.
- If you have Micron SPI NAND chip say yes.
-
- If unsure, say no here.
-
-config MTD_SPINAND_ONDIEECC
- bool "Use SPINAND internal ECC"
- depends on MTD_SPINAND_MT29F
- help
- Internal ECC.
- Enables Hardware ECC support for Micron SPI NAND.
diff --git a/drivers/staging/mt29f_spinand/Makefile
b/drivers/staging/mt29f_spinand/Makefile
deleted file mode 100644
index e47af0f7fda9..
--- a/drivers/staging/mt29f_spinand/Makefile
+++ /dev/null
@@ -1 +0,0 @@
-obj-$(CONFIG_MTD_SPINAND_MT29F) += mt29f_spinand.o
diff --git a/drivers/staging/mt29f_spinand/TODO
b/drivers/staging/mt29f_spinand/TODO
deleted file mode 100644
index a2209b72d371..
--- a/drivers/staging/mt29f_spinand/TODO
+++ /dev/null
@@ -1,13 +0,0 @@
-TODO:
- - Tested on XLP platform, needs to be tested on other platforms.
- - Checkpatch.pl cleanups
- - Sparce fixes.
- - Clean up coding style to meet kernel standard.
-
-Please send patches
-To:
-Kamlakant Patel
-Cc:
-Greg Kroah-Hartman
-Mona Anonuevo
-linux-...@lists.infradead.org
diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c
b/drivers/staging/mt29f_spinand/mt29f_spinand.c
deleted file mode 100644
index def8a1f57d1c..
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ /dev/null
@@ -1,980 +0,0 @@
-/*
- * Copyright (c) 2003-2013 Broadcom Corporation
- *
- * Copyright (c) 2009-2010 Micron Technology, Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include "mt29f_spinand.h"
-
-#define BUFSIZE (10 * 64 * 2048)
-#define CACHE_BUF 2112
-/*
- * OOB area specification layout: Total 32 available free bytes.
- */
-
-static inline struct spinand_state *mtd_to_state(struct mtd_info *mtd)
-{
- struct
[PATCH v2] staging: bcm2835-audio: interpolate audio delay
When the BCM2835 audio output is used, userspace sees a jitter up to 10ms
in the audio position, aka "delay" -- the number of frames that must
be output before a new frame would be played.
Make this a bit nicer for userspace by interpolating the position
using the CPU clock.
The overhead is small -- an extra ktime_get() every time a GPU message
is sent -- and another call and a few calculations whenever the delay
is sought from userland.
At 48,000 frames per second, i.e. approximately 20 microseconds per
frame, it would take a clock inaccuracy of
20 microseconds in 10 milliseconds -- 2,000 parts per million --
to result in an inaccurate estimate, whereas
crystal- or resonator-based clocks typically have an
inaccuracy of 10s to 100s of parts per million.
Signed-off-by: Mike Brady
---
Changes in v2 -- remove inappropriate addition of SNDRV_PCM_INFO_BATCH flag
.../vc04_services/bcm2835-audio/bcm2835-pcm.c | 20 +++
.../vc04_services/bcm2835-audio/bcm2835.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
index e66da11af5cf..9053b996cada 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
@@ -74,6 +74,7 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream
*alsa_stream,
atomic_set(_stream->pos, pos);
alsa_stream->period_offset += bytes;
+ alsa_stream->interpolate_start = ktime_get();
if (alsa_stream->period_offset >= alsa_stream->period_size) {
alsa_stream->period_offset %= alsa_stream->period_size;
snd_pcm_period_elapsed(substream);
@@ -243,6 +244,7 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream
*substream)
atomic_set(_stream->pos, 0);
alsa_stream->period_offset = 0;
alsa_stream->draining = false;
+ alsa_stream->interpolate_start = ktime_get();
return 0;
}
@@ -292,6 +294,24 @@ snd_bcm2835_pcm_pointer(struct snd_pcm_substream
*substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct bcm2835_alsa_stream *alsa_stream = runtime->private_data;
+ ktime_t now = ktime_get();
+
+ /* Give userspace better delay reporting by interpolating between GPU
+* notifications, assuming audio speed is close enough to the clock
+* used for ktime
+*/
+
+ if ((ktime_to_ns(alsa_stream->interpolate_start)) &&
+ (ktime_compare(alsa_stream->interpolate_start, now) < 0)) {
+ u64 interval =
+ (ktime_to_ns(ktime_sub(now,
+ alsa_stream->interpolate_start)));
+ u64 frames_output_in_interval =
+ div_u64((interval * runtime->rate), 10);
+ snd_pcm_sframes_t frames_output_in_interval_sized =
+ -frames_output_in_interval;
+ runtime->delay = frames_output_in_interval_sized;
+ }
return snd_pcm_indirect_playback_pointer(substream,
_stream->pcm_indirect,
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
index e13435d1c205..595ad584243f 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
@@ -78,6 +78,7 @@ struct bcm2835_alsa_stream {
unsigned int period_offset;
unsigned int buffer_size;
unsigned int period_size;
+ ktime_t interpolate_start;
struct bcm2835_audio_instance *instance;
int idx;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-audio: interpolate audio delay
Hi Kirill.
The problem that this patch seeks to resolve is that when userland asks for the
delay -- the time to hear -- the driver responds with a figure that is only
updated
when a GPU interrupt occurs, now approximately every 10 milliseconds.
As far as I am aware, there is no way to get a more up-to-date delay number on
request,
hence the present patch to interpolate based on the time of the last update,
the time of
the request and the nominal frame rate.
It does have the drawback of relying on the accuracy of the CPU clock and on the
nominal rate but on reasonable assumptions, it will not be in error over
the approximately 10 ms interval.
As you'll probably know, this is the built-in DAC in the full-sized Raspberry
Pi devices and
is widely used. As a developer of Shairport Sync, I have noticed that the
"slop" in
delay on the Pi's built-in DAC is very large by comparison with other DACs.
FYI, there is
a discussion of the effects of a downstream equivalent of this suggested patch
at:
https://github.com/raspberrypi/firmware/issues/1026#issuecomment-415746016.
Best wishes
Mike
On Thu, Oct 18, 2018 at 09:48:36PM +0200, Kirill Marinushkin wrote:
> Hello Mike,
>
> On 10/18/18 12:57, Mike Brady wrote:
> > + ktime_t now = ktime_get();
> > +
> > + /* Give userspace better delay reporting by interpolating between GPU
> > +* notifications, assuming audio speed is close enough to the clock
> > +* used for ktime
> > +*/
> > +
> > + if ((ktime_to_ns(alsa_stream->interpolate_start)) &&
> > + (ktime_compare(alsa_stream->interpolate_start, now) < 0)) {
> > + u64 interval =
> > + (ktime_to_ns(ktime_sub(now,
> > + alsa_stream->interpolate_start)));
> > + u64 frames_output_in_interval =
> > + div_u64((interval * runtime->rate), 10);
> > + snd_pcm_sframes_t frames_output_in_interval_sized =
> > + -frames_output_in_interval;
> > + runtime->delay = frames_output_in_interval_sized;
> > + }
>
> This doesn't look like a good solution for me. More like a workaround. What is
> the root cause of the delay?
>
> Best Regards,
> Kirill
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/6] staging: vboxvideo: Fixup some #ifdef-s
Hi, On 22-10-18 17:42, Emil Velikov wrote: On Mon, 22 Oct 2018 at 15:58, Hans de Goede wrote: Add #ifdef CONFIG_PM_SLEEP around the suspend/hibernate functions. Remove unnecessary #ifdef CONFIG_COMPAT, the .compat_ioctl member is always available and if CONFIG_COMPAT is not set then drm_compat_ioctl is defined to NULL. Signed-off-by: Hans de Goede --- drivers/staging/vboxvideo/vbox_drv.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c index f9f4c6c2a4e9..b6e6530aa4be 100644 --- a/drivers/staging/vboxvideo/vbox_drv.c +++ b/drivers/staging/vboxvideo/vbox_drv.c @@ -117,6 +117,7 @@ static void vbox_pci_remove(struct pci_dev *pdev) drm_dev_put(>ddev); } +#ifdef CONFIG_PM_SLEEP static int vbox_pm_suspend(struct device *dev) AFAICT Arnd has been annotating these as __maybe_unused That only works in combinations with a macro like SET_SYSTEM_SLEEP_PM_OPS(), which makes sure that we do not actually set the .suspend, etc. members of dev_pm_ops when CONFIG_PM_SLEEP is not set, so that the code actually gets compiled out. We cannot use SET_SYSTEM_SLEEP_PM_OPS here, since the suspend and hibernate paths differ. Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: adt7316: Switch to the gpio descriptor interface
On Mon, 2018-10-22 at 22:44 +0530, Nishad Kamdar wrote:
> Use the gpiod interface instead of the deprecated old non-descriptor
> interface for ldac_pin.
>
> Signed-off-by: Nishad Kamdar
> ---
Hi Nishad,
I have been working on implementing device tree bindings for this
driver and removing platform data from it.
I've sent a series of patches for it to Jonathan. I didn't send it
on the mailing list as I wanted to confirm from Jonathan if we wants
something more to be done in the series. That series also includes
the change which you are proposing here.
I am really sorry that you didn't know about this.
If you are planning to send any other patches for this driver then
let me know before you start implementing it so that we can
co-ordinate before doing any work.
Thanks
> drivers/staging/iio/addac/adt7316.c | 17 -
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/addac/adt7316.c
> b/drivers/staging/iio/addac/adt7316.c
> index 3f22d1088713..94f945ba0097 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -8,7 +8,7 @@
> */
>
> #include
> -#include
> +#include
> #include
> #include
> #include
> @@ -177,7 +177,7 @@
>
> struct adt7316_chip_info {
> struct adt7316_bus bus;
> - u16 ldac_pin;
> + struct gpio_desc*ldac_pin;
> u16 int_mask; /* 0x2f */
> u8 config1;
> u8 config2;
> @@ -950,8 +950,8 @@ static ssize_t adt7316_store_update_DAC(struct
> device *dev,
> if (ret)
> return -EIO;
> } else {
> - gpio_set_value(chip->ldac_pin, 0);
> - gpio_set_value(chip->ldac_pin, 1);
> + gpiod_set_value(chip->ldac_pin, 0);
> + gpiod_set_value(chip->ldac_pin, 1);
> }
>
> return len;
> @@ -2120,7 +2120,14 @@ int adt7316_probe(struct device *dev, struct
> adt7316_bus *bus,
> else
> return -ENODEV;
>
> - chip->ldac_pin = adt7316_platform_data[1];
> + chip->ldac_pin = devm_gpiod_get(dev, "ldac",
> GPIOD_OUT_HIGH);
> + if (IS_ERR(chip->ldac_pin)) {
> + ret = PTR_ERR(chip->ldac_pin);
> + dev_err(dev, "Failed to request ldac GPIO: %d\n",
> + ret);
> + return ret;
> + }
> +
> if (chip->ldac_pin) {
> chip->config3 |= ADT7316_DA_EN_VIA_DAC_LDCA;
> if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: adt7316: Switch to the gpio descriptor interface
Use the gpiod interface instead of the deprecated old non-descriptor
interface for ldac_pin.
Signed-off-by: Nishad Kamdar
---
drivers/staging/iio/addac/adt7316.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 3f22d1088713..94f945ba0097 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -8,7 +8,7 @@
*/
#include
-#include
+#include
#include
#include
#include
@@ -177,7 +177,7 @@
struct adt7316_chip_info {
struct adt7316_bus bus;
- u16 ldac_pin;
+ struct gpio_desc*ldac_pin;
u16 int_mask; /* 0x2f */
u8 config1;
u8 config2;
@@ -950,8 +950,8 @@ static ssize_t adt7316_store_update_DAC(struct device *dev,
if (ret)
return -EIO;
} else {
- gpio_set_value(chip->ldac_pin, 0);
- gpio_set_value(chip->ldac_pin, 1);
+ gpiod_set_value(chip->ldac_pin, 0);
+ gpiod_set_value(chip->ldac_pin, 1);
}
return len;
@@ -2120,7 +2120,14 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
else
return -ENODEV;
- chip->ldac_pin = adt7316_platform_data[1];
+ chip->ldac_pin = devm_gpiod_get(dev, "ldac", GPIOD_OUT_HIGH);
+ if (IS_ERR(chip->ldac_pin)) {
+ ret = PTR_ERR(chip->ldac_pin);
+ dev_err(dev, "Failed to request ldac GPIO: %d\n",
+ ret);
+ return ret;
+ }
+
if (chip->ldac_pin) {
chip->config3 |= ADT7316_DA_EN_VIA_DAC_LDCA;
if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: iio: ad2s1210: Switch to the gpio descriptor interface
Use the gpiod interface instead of the deprecated old non-descriptor
interface.
Signed-off-by: Nishad Kamdar
---
Changes in v2:
- Use the spi_device struct embedded in st instead
of passing it as an argument to ad2s1210_setup_gpios().
- Use an array of structs to reduce redundant code in
in ad2s1210_setup_gpios().
- Remove ad2s1210_free_gpios() as devm API is being used.
---
drivers/staging/iio/resolver/ad2s1210.c | 91 +
drivers/staging/iio/resolver/ad2s1210.h | 3 -
2 files changed, 48 insertions(+), 46 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c
b/drivers/staging/iio/resolver/ad2s1210.c
index ac13b99bd9cb..e4c222b47574 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -15,7 +15,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -69,10 +69,21 @@ enum ad2s1210_mode {
static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
+struct ad2s1210_gpio {
+ struct gpio_desc *ptr;
+ char *name;
+ unsigned long flags;
+};
+
struct ad2s1210_state {
const struct ad2s1210_platform_data *pdata;
struct mutex lock;
struct spi_device *sdev;
+ struct gpio_desc *sample;
+ struct gpio_desc *a0;
+ struct gpio_desc *a1;
+ struct gpio_desc *res0;
+ struct gpio_desc *res1;
unsigned int fclkin;
unsigned int fexcit;
bool hysteresis;
@@ -91,8 +102,8 @@ static const int ad2s1210_mode_vals[4][2] = {
static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
struct ad2s1210_state *st)
{
- gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
- gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
+ gpiod_set_value(st->a0, ad2s1210_mode_vals[mode][0]);
+ gpiod_set_value(st->a1, ad2s1210_mode_vals[mode][1]);
st->mode = mode;
}
@@ -152,8 +163,8 @@ int ad2s1210_update_frequency_control_word(struct
ad2s1210_state *st)
static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
{
- int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
- gpio_get_value(st->pdata->res[1]);
+ int resolution = (gpiod_get_value(st->res0) << 1) |
+ gpiod_get_value(st->res1);
return ad2s1210_resolution_value[resolution];
}
@@ -164,10 +175,10 @@ static const int ad2s1210_res_pins[4][2] = {
static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
{
- gpio_set_value(st->pdata->res[0],
- ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
- gpio_set_value(st->pdata->res[1],
- ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
+ gpiod_set_value(st->res0,
+ ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
+ gpiod_set_value(st->res1,
+ ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
}
static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
@@ -401,15 +412,15 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
int ret;
mutex_lock(>lock);
- gpio_set_value(st->pdata->sample, 0);
+ gpiod_set_value(st->sample, 0);
/* delay (2 * tck + 20) nano seconds */
udelay(1);
- gpio_set_value(st->pdata->sample, 1);
+ gpiod_set_value(st->sample, 1);
ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
if (ret < 0)
goto error_ret;
- gpio_set_value(st->pdata->sample, 0);
- gpio_set_value(st->pdata->sample, 1);
+ gpiod_set_value(st->sample, 0);
+ gpiod_set_value(st->sample, 1);
error_ret:
mutex_unlock(>lock);
@@ -466,7 +477,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
s16 vel;
mutex_lock(>lock);
- gpio_set_value(st->pdata->sample, 0);
+ gpiod_set_value(st->sample, 0);
/* delay (6 * tck + 20) nano seconds */
udelay(1);
@@ -512,7 +523,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
}
error_ret:
- gpio_set_value(st->pdata->sample, 1);
+ gpiod_set_value(st->sample, 1);
/* delay (2 * tck + 20) nano seconds */
udelay(1);
mutex_unlock(>lock);
@@ -630,30 +641,30 @@ static const struct iio_info ad2s1210_info = {
static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
{
- unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
- struct gpio ad2s1210_gpios[] = {
- { st->pdata->sample, GPIOF_DIR_IN, "sample" },
- { st->pdata->a[0], flags, "a0" },
- { st->pdata->a[1], flags, "a1" },
- { st->pdata->res[0], flags, "res0" },
- { st->pdata->res[0], flags, "res1" },
+ int ret = 0, i = 0;
+ struct spi_device *spi = st->sdev;
+ unsigned long
Re: [PATCH] staging: iio: ad2s1210: Switch to the gpio descriptor interface
On Sun, Oct 21, 2018 at 10:31:38AM +0200, Slawomir Stepien wrote:
> On paź 21, 2018 11:49, Nishad Kamdar wrote:
> > Use the gpiod interface instead of the deprecated old non-descriptor
> > interface.
>
> Hi!
>
> See my comments below.
>
> > Signed-off-by: Nishad Kamdar
> > ---
> > drivers/staging/iio/resolver/ad2s1210.c | 110 +++-
> > drivers/staging/iio/resolver/ad2s1210.h | 3 -
> > 2 files changed, 71 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s1210.c
> > b/drivers/staging/iio/resolver/ad2s1210.c
> > index ac13b99bd9cb..ed372a5f9130 100644
> > --- a/drivers/staging/iio/resolver/ad2s1210.c
> > +++ b/drivers/staging/iio/resolver/ad2s1210.c
> > @@ -15,7 +15,7 @@
> > #include
> > #include
> > #include
> > -#include
> > +#include
> > #include
> >
> > #include
> > @@ -73,6 +73,11 @@ struct ad2s1210_state {
> > const struct ad2s1210_platform_data *pdata;
> > struct mutex lock;
> > struct spi_device *sdev;
> > + struct gpio_desc *sample;
> > + struct gpio_desc *a0;
> > + struct gpio_desc *a1;
> > + struct gpio_desc *res0;
> > + struct gpio_desc *res1;
> > unsigned int fclkin;
> > unsigned int fexcit;
> > bool hysteresis;
> > @@ -91,8 +96,8 @@ static const int ad2s1210_mode_vals[4][2] = {
> > static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
> > struct ad2s1210_state *st)
> > {
> > - gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
> > - gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
> > + gpiod_set_value(st->a0, ad2s1210_mode_vals[mode][0]);
> > + gpiod_set_value(st->a1, ad2s1210_mode_vals[mode][1]);
> > st->mode = mode;
> > }
> >
> > @@ -152,8 +157,8 @@ int ad2s1210_update_frequency_control_word(struct
> > ad2s1210_state *st)
> >
> > static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state
> > *st)
> > {
> > - int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
> > - gpio_get_value(st->pdata->res[1]);
> > + int resolution = (gpiod_get_value(st->res0) << 1) |
> > + gpiod_get_value(st->res1);
> >
> > return ad2s1210_resolution_value[resolution];
> > }
> > @@ -164,10 +169,10 @@ static const int ad2s1210_res_pins[4][2] = {
> >
> > static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
> > {
> > - gpio_set_value(st->pdata->res[0],
> > - ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
> > - gpio_set_value(st->pdata->res[1],
> > - ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
> > + gpiod_set_value(st->res0,
> > + ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
> > + gpiod_set_value(st->res1,
> > + ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
> > }
> >
> > static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
> > @@ -401,15 +406,15 @@ static ssize_t ad2s1210_clear_fault(struct device
> > *dev,
> > int ret;
> >
> > mutex_lock(>lock);
> > - gpio_set_value(st->pdata->sample, 0);
> > + gpiod_set_value(st->sample, 0);
> > /* delay (2 * tck + 20) nano seconds */
> > udelay(1);
> > - gpio_set_value(st->pdata->sample, 1);
> > + gpiod_set_value(st->sample, 1);
> > ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
> > if (ret < 0)
> > goto error_ret;
> > - gpio_set_value(st->pdata->sample, 0);
> > - gpio_set_value(st->pdata->sample, 1);
> > + gpiod_set_value(st->sample, 0);
> > + gpiod_set_value(st->sample, 1);
> > error_ret:
> > mutex_unlock(>lock);
> >
> > @@ -466,7 +471,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
> > s16 vel;
> >
> > mutex_lock(>lock);
> > - gpio_set_value(st->pdata->sample, 0);
> > + gpiod_set_value(st->sample, 0);
> > /* delay (6 * tck + 20) nano seconds */
> > udelay(1);
> >
> > @@ -512,7 +517,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
> > }
> >
> > error_ret:
> > - gpio_set_value(st->pdata->sample, 1);
> > + gpiod_set_value(st->sample, 1);
> > /* delay (2 * tck + 20) nano seconds */
> > udelay(1);
> > mutex_unlock(>lock);
> > @@ -628,32 +633,59 @@ static const struct iio_info ad2s1210_info = {
> > .attrs = _attribute_group,
> > };
> >
> > -static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
> > +static int ad2s1210_setup_gpios(struct spi_device *spi,
> > + struct ad2s1210_state *st)
>
> This change is not needed. The st has the spi_device inside. Use
> container_of()
> macro here.
>
> > {
> > - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
> > - struct gpio ad2s1210_gpios[] = {
> > - { st->pdata->sample, GPIOF_DIR_IN, "sample" },
> > - { st->pdata->a[0], flags, "a0" },
> > - { st->pdata->a[1], flags, "a1" },
> > - { st->pdata->res[0],
Re: [PATCH 3/6] staging: vboxvideo: Fixup some #ifdef-s
On Mon, 22 Oct 2018 at 15:58, Hans de Goede wrote: > > Add #ifdef CONFIG_PM_SLEEP around the suspend/hibernate functions. > > Remove unnecessary #ifdef CONFIG_COMPAT, the .compat_ioctl member is > always available and if CONFIG_COMPAT is not set then drm_compat_ioctl > is defined to NULL. > > Signed-off-by: Hans de Goede > --- > drivers/staging/vboxvideo/vbox_drv.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/vboxvideo/vbox_drv.c > b/drivers/staging/vboxvideo/vbox_drv.c > index f9f4c6c2a4e9..b6e6530aa4be 100644 > --- a/drivers/staging/vboxvideo/vbox_drv.c > +++ b/drivers/staging/vboxvideo/vbox_drv.c > @@ -117,6 +117,7 @@ static void vbox_pci_remove(struct pci_dev *pdev) > drm_dev_put(>ddev); > } > > +#ifdef CONFIG_PM_SLEEP > static int vbox_pm_suspend(struct device *dev) AFAICT Arnd has been annotating these as __maybe_unused -Emil ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging: vboxvideo: Add myself as MODULE_AUTHOR
Add myself as MODULE_AUTHOR.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index b6e6530aa4be..80d7395e48dc 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -277,5 +277,6 @@ module_init(vbox_init);
module_exit(vbox_exit);
MODULE_AUTHOR("Oracle Corporation");
+MODULE_AUTHOR("Hans de Goede ");
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL and additional rights");
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging: vboxvideo: Cleanup header use
Stop adding -Iinclude/drm to the CFLAGS and stop using the obsolete
"include-all" drm/drmP.h header.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/Makefile | 2 --
drivers/staging/vboxvideo/vbox_drv.c | 1 -
drivers/staging/vboxvideo/vbox_drv.h | 2 +-
drivers/staging/vboxvideo/vbox_fb.c | 1 -
drivers/staging/vboxvideo/vbox_ttm.c | 4 +++-
5 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vboxvideo/Makefile
b/drivers/staging/vboxvideo/Makefile
index 3f6094aa9cdf..1224f313af0c 100644
--- a/drivers/staging/vboxvideo/Makefile
+++ b/drivers/staging/vboxvideo/Makefile
@@ -1,6 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-ccflags-y := -Iinclude/drm
-
vboxvideo-y := hgsmi_base.o modesetting.o vbva_base.o \
vbox_drv.o vbox_fb.o vbox_hgsmi.o vbox_irq.o vbox_main.o \
vbox_mode.o vbox_prime.o vbox_ttm.o
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index d7440b9e5eea..9997beac00fb 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -11,7 +11,6 @@
#include
#include
-#include
#include
#include "vbox_drv.h"
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index f82e594eca4b..cb0bc9201a7f 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -12,10 +12,10 @@
#include
#include
+#include
#include
#include
-#include
#include
#include
#include
diff --git a/drivers/staging/vboxvideo/vbox_fb.c
b/drivers/staging/vboxvideo/vbox_fb.c
index 8041d0c46a6b..af224eb5ebc1 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -17,7 +17,6 @@
#include
#include
-#include
#include
#include
#include
diff --git a/drivers/staging/vboxvideo/vbox_ttm.c
b/drivers/staging/vboxvideo/vbox_ttm.c
index 74f2d271ddac..edc8686454bf 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -6,8 +6,10 @@
* Authors: Dave Airlie
* Michael Thayer
*/
+#include
+#include
+#include
#include "vbox_drv.h"
-#include
static inline struct vbox_private *vbox_bdev(struct ttm_bo_device *bd)
{
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: vboxvideo: Drop DRM_MODE_FB_CMD define
This is a leftover from when the driver was out of tree and also build
against older kernels.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.h | 4 +---
drivers/staging/vboxvideo/vbox_fb.c | 2 +-
drivers/staging/vboxvideo/vbox_main.c | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index cb0bc9201a7f..e2a6b2b5bfc3 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -164,8 +164,6 @@ void vbox_hw_fini(struct vbox_private *vbox);
int vbox_mode_init(struct vbox_private *vbox);
void vbox_mode_fini(struct vbox_private *vbox);
-#define DRM_MODE_FB_CMD drm_mode_fb_cmd2
-
void vbox_report_caps(struct vbox_private *vbox);
void vbox_framebuffer_dirty_rectangles(struct drm_framebuffer *fb,
@@ -174,7 +172,7 @@ void vbox_framebuffer_dirty_rectangles(struct
drm_framebuffer *fb,
int vbox_framebuffer_init(struct vbox_private *vbox,
struct vbox_framebuffer *vbox_fb,
- const struct DRM_MODE_FB_CMD *mode_cmd,
+ const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj);
int vboxfb_create(struct drm_fb_helper *helper,
diff --git a/drivers/staging/vboxvideo/vbox_fb.c
b/drivers/staging/vboxvideo/vbox_fb.c
index af224eb5ebc1..d3a5255c93b6 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -51,7 +51,7 @@ int vboxfb_create(struct drm_fb_helper *helper,
struct vbox_private *vbox =
container_of(helper, struct vbox_private, fb_helper);
struct pci_dev *pdev = vbox->ddev.pdev;
- struct DRM_MODE_FB_CMD mode_cmd;
+ struct drm_mode_fb_cmd2 mode_cmd;
struct drm_framebuffer *fb;
struct fb_info *info;
struct drm_gem_object *gobj;
diff --git a/drivers/staging/vboxvideo/vbox_main.c
b/drivers/staging/vboxvideo/vbox_main.c
index 1328f82a083d..8348e71c4c9d 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -103,7 +103,7 @@ static const struct drm_framebuffer_funcs vbox_fb_funcs = {
int vbox_framebuffer_init(struct vbox_private *vbox,
struct vbox_framebuffer *vbox_fb,
- const struct DRM_MODE_FB_CMD *mode_cmd,
+ const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj)
{
int ret;
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: vboxvideo: Use PCI_DEVICE() for our pci_id table
Use PCI_DEVICE() for our pci_id table and also simplify the terminating
entry tio just "{ }".
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index 9997beac00fb..f9f4c6c2a4e9 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -23,8 +23,8 @@ module_param_named(modeset, vbox_modeset, int, 0400);
static struct drm_driver driver;
static const struct pci_device_id pciidlist[] = {
- { 0x80ee, 0xbeef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
- { 0, 0, 0},
+ { PCI_DEVICE(0x80ee, 0xbeef) },
+ { }
};
MODULE_DEVICE_TABLE(pci, pciidlist);
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: vboxvideo: Use DRM_FB_HELPER_DEFAULT_OPS
Use DRM_FB_HELPER_DEFAULT_OPS rather then open-coding it.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_fb.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_fb.c
b/drivers/staging/vboxvideo/vbox_fb.c
index d3a5255c93b6..8a2f2e8b57c1 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -33,16 +33,10 @@ static struct fb_deferred_io vbox_defio = {
static struct fb_ops vboxfb_ops = {
.owner = THIS_MODULE,
- .fb_check_var = drm_fb_helper_check_var,
- .fb_set_par = drm_fb_helper_set_par,
+ DRM_FB_HELPER_DEFAULT_OPS,
.fb_fillrect = drm_fb_helper_sys_fillrect,
.fb_copyarea = drm_fb_helper_sys_copyarea,
.fb_imageblit = drm_fb_helper_sys_imageblit,
- .fb_pan_display = drm_fb_helper_pan_display,
- .fb_blank = drm_fb_helper_blank,
- .fb_setcmap = drm_fb_helper_setcmap,
- .fb_debug_enter = drm_fb_helper_debug_enter,
- .fb_debug_leave = drm_fb_helper_debug_leave,
};
int vboxfb_create(struct drm_fb_helper *helper,
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging: vboxvideo: Fixup some #ifdef-s
Add #ifdef CONFIG_PM_SLEEP around the suspend/hibernate functions.
Remove unnecessary #ifdef CONFIG_COMPAT, the .compat_ioctl member is
always available and if CONFIG_COMPAT is not set then drm_compat_ioctl
is defined to NULL.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index f9f4c6c2a4e9..b6e6530aa4be 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -117,6 +117,7 @@ static void vbox_pci_remove(struct pci_dev *pdev)
drm_dev_put(>ddev);
}
+#ifdef CONFIG_PM_SLEEP
static int vbox_pm_suspend(struct device *dev)
{
struct vbox_private *vbox = dev_get_drvdata(dev);
@@ -172,13 +173,16 @@ static const struct dev_pm_ops vbox_pm_ops = {
.poweroff = vbox_pm_poweroff,
.restore = vbox_pm_resume,
};
+#endif
static struct pci_driver vbox_pci_driver = {
.name = DRIVER_NAME,
.id_table = pciidlist,
.probe = vbox_pci_probe,
.remove = vbox_pci_remove,
+#ifdef CONFIG_PM_SLEEP
.driver.pm = _pm_ops,
+#endif
};
static const struct file_operations vbox_fops = {
@@ -186,11 +190,9 @@ static const struct file_operations vbox_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = drm_ioctl,
+ .compat_ioctl = drm_compat_ioctl,
.mmap = vbox_mmap,
.poll = drm_poll,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = drm_compat_ioctl,
-#endif
.read = drm_read,
};
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] staging: vboxvideo: Address various review remarks
Hi All, Note this series applies on top of my serious from a couple of days ago which starts with "[PATCH 1/6] staging: vboxvideo: Stop accessing crtc_state->active" Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On 10/22/18 17:32, Laura Abbott wrote:
> On 10/22/2018 07:02 AM, Alexey Skidanov wrote:
>> The heap name might be uninitialized and access might crash the
>> kernel.
>>
>
> The heap name should never be null so this seems like this is being
> fixed in the wrong place. Can you explain more how you are hitting
> this issue?
Sure. Carve out heap name is uninitialized. There is the next patch
fixing it. But to be on the safe side, I have added the check.
Thanks,
Alexey
>
> Thanks,
> Laura
>
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion.c | 7 +--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..55bca92d 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
>> max_cnt = query->cnt;
>> plist_for_each_entry(heap, >heaps, node) {
>> - strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> - hdata.name[sizeof(hdata.name) - 1] = '\0';
>> + if (heap->name) {
>> + strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> + hdata.name[sizeof(hdata.name) - 1] = '\0';
>> + }
>> +
>> hdata.type = heap->type;
>> hdata.heap_id = heap->id;
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On 10/22/2018 07:02 AM, Alexey Skidanov wrote:
The heap name might be uninitialized and access might crash the
kernel.
The heap name should never be null so this seems like this is being
fixed in the wrong place. Can you explain more how you are hitting
this issue?
Thanks,
Laura
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..55bca92d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
max_cnt = query->cnt;
plist_for_each_entry(heap, >heaps, node) {
- strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
- hdata.name[sizeof(hdata.name) - 1] = '\0';
+ if (heap->name) {
+ strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
+ hdata.name[sizeof(hdata.name) - 1] = '\0';
+ }
+
hdata.type = heap->type;
hdata.heap_id = heap->id;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drm/vboxvideo: Move vboxvideo driver out of staging
Hi,
Thank you for the review.
On 18-10-18 20:12, Sam Ravnborg wrote:
Hi Hans.
Just a bunch of random observations that I hope you find use of.
Sam
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index cb88528e7b10..6b4d6c957da8 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -315,6 +315,8 @@ source "drivers/gpu/drm/tve200/Kconfig"
source "drivers/gpu/drm/xen/Kconfig"
+source "drivers/gpu/drm/vboxvideo/Kconfig"
+
# Keep legacy drivers last
menuconfig DRM_LEGACY
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index a6771cef85e2..133606802300 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -107,3 +107,4 @@ obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
obj-$(CONFIG_DRM_PL111) += pl111/
obj-$(CONFIG_DRM_TVE200) += tve200/
obj-$(CONFIG_DRM_XEN) += xen/
+obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
Is it the most logical place to add this in the end?
You are asking for merge problems if you do so, but then
the drm drivers does not seem to follow any good order
so this may be fine.
Right, just tagging the entry to the end seems to be what
all new driver additions do, so I'm just following that
example here.
+++ b/drivers/gpu/drm/vboxvideo/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+ccflags-y := -Iinclude/drm
This should not be required.
If required then fix the offending #include
Fixed in the set of cleanup patches which I'm sending out
after this email.
Note these cleanup patches apply to the staging version of
the driver, I will do a new version of this patch after
someone has found the time to do a more thorough review.
+
+vboxvideo-y := hgsmi_base.o modesetting.o vbva_base.o \
+ vbox_drv.o vbox_fb.o vbox_hgsmi.o vbox_irq.o vbox_main.o \
+ vbox_mode.o vbox_prime.o vbox_ttm.o
+
+obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo.o
diff --git a/drivers/gpu/drm/vboxvideo/hgsmi_base.c
b/drivers/gpu/drm/vboxvideo/hgsmi_base.c
new file mode 100644
index ..361d3193258e
--- /dev/null
+++ b/drivers/gpu/drm/vboxvideo/hgsmi_base.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: MIT
+/* Copyright (C) 2006-2017 Oracle Corporation */
2018?
General comment for all files.
I do not work for Oracle, so its not up to me to extend the copyright
claim.
+ union {
+ /* Opaque placeholder to make the union 8 bytes. */
+ u8 header_data[8];
Further down you have 2 x u32, so the union is guaranteed to be 64 bits.
So header_data is redundant.
The header_data member is used in the code.
+
+ /* HGSMI_BUFFER_HEADER_F_SEQ_SINGLE */
+ struct {
+ u32 reserved1; /* A reserved field, initialize to 0. */
+ u32 reserved2; /* A reserved field, initialize to 0. */
+ } buffer;
--- /dev/null
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -0,0 +1,280 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright (C) 2013-2017 Oracle Corporation
+ * This file is based on ast_drv.c
+ * Copyright 2012 Red Hat Inc.
+ * Authors: Dave Airlie
+ * Michael Thayer
+ */
+#include
+#include
+#include
+
+#include
We are trying to get rid of drmP - replace with more specific includes.
Goes for all uses of drmP.h
Fixed in the cleanup series.
+
+static int vbox_modeset = -1;
+
+MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
+module_param_named(modeset, vbox_modeset, int, 0400);
+
+static struct drm_driver driver;
+
+static const struct pci_device_id pciidlist[] = {
+ { 0x80ee, 0xbeef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+ { 0, 0, 0},
+};
Use PCI_DEVICE()?
Ack, fixed in the cleanup series.
+static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id
*ent)
+{
+ struct vbox_private *vbox;
+ int ret = 0;
+
+ if (!vbox_check_supported(VBE_DISPI_ID_HGSMI))
+ return -ENODEV;
+
+ vbox = kzalloc(sizeof(*vbox), GFP_KERNEL);
+ if (!vbox)
+ return -ENOMEM;
+
+ ret = drm_dev_init(>ddev, , >dev);
+ if (ret) {
+ kfree(vbox);
+ return ret;
+ }
+
+ vbox->ddev.pdev = pdev;
+ vbox->ddev.dev_private = vbox;
+ pci_set_drvdata(pdev, vbox);
+ mutex_init(>hw_mutex);
+
+ ret = pci_enable_device(pdev);
+ if (ret)
+ goto err_dev_put;
+
+ ret = vbox_hw_init(vbox);
+ if (ret)
+ goto err_pci_disable;
+
+ ret = vbox_mm_init(vbox);
+ if (ret)
+ goto err_hw_fini;
+
+ ret = vbox_mode_init(vbox);
+ if (ret)
+ goto err_mm_fini;
+
+ ret = vbox_irq_init(vbox);
+ if (ret)
+ goto err_mode_fini;
+
+ ret = drm_fb_helper_fbdev_setup(>ddev, >fb_helper,
+ _fb_helper_funcs, 32,
+ vbox->num_crtcs);
+ if (ret)
+
[PATCH] staging: android: ion: Fixed uninitialized heap name access
The heap name might be uninitialized and access might crash the
kernel.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..55bca92d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
max_cnt = query->cnt;
plist_for_each_entry(heap, >heaps, node) {
- strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
- hdata.name[sizeof(hdata.name) - 1] = '\0';
+ if (heap->name) {
+ strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
+ hdata.name[sizeof(hdata.name) - 1] = '\0';
+ }
+
hdata.type = heap->type;
hdata.heap_id = heap->id;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/15] Add support for OLPC XO 1.75 Embedded Controller
Hi, On Wed, Oct 10, 2018 at 07:22:45PM +0200, Lubomir Rintel wrote: > Hi. > > This patchset adds support for the Embedded Controller on an OLPC XO > 1.75 machine. OLPC XO 1.75 is a MMP2 based ARM laptop. It plugs into > the existing OLPC platform infrastructure, currently used by the x86 > based models. > > The EC operates in SPI master mode, meaning the SOC is the SPI slave. It > uses extra handshake signal to signal readiness of SOC to accept data > from EC and to initiate a transaction if SOC wishes to submit data. > > The SPI slave support for MMP2 was submitted separately: > https://lore.kernel.org/lkml/20181010170936.316862-1-lkund...@v3.sk/T/#t > > THe "power: supply: olpc_battery: correct the temperature" patch was > already sent out separately, but I'm including it because the last > commit of the set depends on it. > > Tested to work on an OLPC XO 1.75 and also tested not to break x86 > support with an OLPC XO 1 machine. I don't have a XO 1.5, but it's > unlikely this breaks it when XO 1 works. > > Thanks in advance for reviews and feedback of any kind. I reviewed the power-supply related patches (1, 10, 12-15) and think they are fine apart from the things found by Andy Shevchenko. -- Sebastian signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/15] power: supply: olpc_battery: correct the temperature units
Hi,
On Fri, Oct 19, 2018 at 04:00:32PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 10, 2018 at 8:23 PM Lubomir Rintel wrote:
> >
> > According to [1] and [2], the temperature values are in tenths of degree
> > Celsius. Exposing the Celsius value makes the battery appear on fire:
> >
> > $ upower -i /org/freedesktop/UPower/devices/battery_olpc_battery
> > ...
> > temperature: 236.9 degrees C
> >
> > Tested on OLPC XO-1 and OLPC XO-1.75 laptops.
>
> It's interesting that the very author of that code is not included in
> so-o long Cc list :)
> Cc: David.
>
> David, do you remember if and how you had tested temperature report of
> the battery on OLPC? I guess this kind of error would be appear immediately.
It depends on the way of testing. It's not so obvious when you just
do a `cat /sys/class/power_supply/.../temperature`, since you just
get the raw integer.
> OTOH it might be that power framework had changed requirements (which
> would be noticeable change).
As far as I know, power-supply has always used 1/10 °C. People tend
to get units wrong all the time though (i.e. mV instead of uV). I'm
not surprised, that this sneaked into the kernel.
> If the latter is true, this patch misses Fixes tag. Actually in any
> case it misses it.
Yes, this should probably have Fixes: fb972873a767 ("[BATTERY] One Laptop
Per Child power/battery driver").
-- Sebastian
> > [1] include/linux/power_supply.h
> > [2] Documentation/power/power_supply_class.txt
> >
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Lubomir Rintel
> > ---
> > drivers/power/supply/olpc_battery.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/power/supply/olpc_battery.c
> > b/drivers/power/supply/olpc_battery.c
> > index 6da79ae14860..5a97e42a3547 100644
> > --- a/drivers/power/supply/olpc_battery.c
> > +++ b/drivers/power/supply/olpc_battery.c
> > @@ -428,14 +428,14 @@ static int olpc_bat_get_property(struct power_supply
> > *psy,
> > if (ret)
> > return ret;
> >
> > - val->intval = (s16)be16_to_cpu(ec_word) * 100 / 256;
> > + val->intval = (s16)be16_to_cpu(ec_word) * 10 / 256;
> > break;
> > case POWER_SUPPLY_PROP_TEMP_AMBIENT:
> > ret = olpc_ec_cmd(EC_AMB_TEMP, NULL, 0, (void *)_word,
> > 2);
> > if (ret)
> > return ret;
> >
> > - val->intval = (int)be16_to_cpu(ec_word) * 100 / 256;
> > + val->intval = (int)be16_to_cpu(ec_word) * 10 / 256;
> > break;
> > case POWER_SUPPLY_PROP_CHARGE_COUNTER:
> > ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void *)_word, 2);
> > --
> > 2.19.0
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/2] memory_hotplug: Free pages as higher order
On 2018-10-19 13:37, Michal Hocko wrote: On Thu 18-10-18 19:18:25, Andrew Morton wrote: [...] So this patch needs more work, yes? Yes, I've talked to Arun (he is offline until next week) offlist and he will play with this some more. Converted totalhigh_pages, totalram_pages and zone->managed_page to atomic and tested hot add. Latency is not effected with this change. Will send out a separate patch on top of this one. Regards, Arun ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
