Re: [PATCH] backlight: led_bl: Add support for an "enable" GPIO
Le Tue, Nov 02, 2021 at 11:25:14AM +, Daniel Thompson a écrit : > On Tue, Nov 02, 2021 at 11:19:42AM +, Daniel Thompson wrote: > > On Tue, Nov 02, 2021 at 10:04:55AM +, Corentin LABBE wrote: > > > From: Jean-Jacques Hiblot > > > > > > This patch adds support for an "enable GPIO". > > > > Before taking this kind of change is there a good reason why backlight > > should manage the GPIO? I thought that the LED subsystem was a sub > > system for LEDs (not LED controllers). In other words if you direct > > that the LED be lit up then isn't it the LED driver's job to manage > > the GPIO and ensure that it lights up? > > Sorry... I should have paid more attention to my sense of déjà vu with > this patch. > > This approach was discussed and rejected when we first introduced the > led_bl driver: > https://lore.kernel.org/linux-leds/20190705100851.zn2jkipj4fxq5we6@devuan/ > Hello I am sorry, I didnt checked if the patch was already submitted or not. Regards
Re: [PATCH] dt-bindings: display: convert faraday,tve200 to YAML
Le Mon, May 17, 2021 at 07:26:24PM -0500, Rob Herring a écrit :
> On Tue, May 11, 2021 at 04:54:48PM +, Corentin Labbe wrote:
> > Converts display/faraday,tve200.txt to yaml.
> >
> > Signed-off-by: Corentin Labbe
> > ---
> > .../bindings/display/faraday,tve200.txt | 54 ---
> > .../bindings/display/faraday,tve200.yaml | 92 +++
> > 2 files changed, 92 insertions(+), 54 deletions(-)
> > delete mode 100644
> > Documentation/devicetree/bindings/display/faraday,tve200.txt
> > create mode 100644
> > Documentation/devicetree/bindings/display/faraday,tve200.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/faraday,tve200.txt
> > b/Documentation/devicetree/bindings/display/faraday,tve200.txt
> > deleted file mode 100644
> > index 82e3bc0b7485..
> > --- a/Documentation/devicetree/bindings/display/faraday,tve200.txt
> > +++ /dev/null
> > @@ -1,54 +0,0 @@
> > -* Faraday TV Encoder TVE200
> > -
> > -Required properties:
> > -
> > -- compatible: must be one of:
> > - "faraday,tve200"
> > - "cortina,gemini-tvc", "faraday,tve200"
> > -
> > -- reg: base address and size of the control registers block
> > -
> > -- interrupts: contains an interrupt specifier for the interrupt
> > - line from the TVE200
> > -
> > -- clock-names: should contain "PCLK" for the clock line clocking the
> > - silicon and "TVE" for the 27MHz clock to the video driver
> > -
> > -- clocks: contains phandle and clock specifier pairs for the entries
> > - in the clock-names property. See
> > - Documentation/devicetree/bindings/clock/clock-bindings.txt
> > -
> > -Optional properties:
> > -
> > -- resets: contains the reset line phandle for the block
> > -
> > -Required sub-nodes:
> > -
> > -- port: describes LCD panel signals, following the common binding
> > - for video transmitter interfaces; see
> > - Documentation/devicetree/bindings/media/video-interfaces.txt
> > - This port should have the properties:
> > - reg = <0>;
> > - It should have one endpoint connected to a remote endpoint where
> > - the display is connected.
> > -
> > -Example:
> > -
> > -display-controller@6a00 {
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > - compatible = "faraday,tve200";
> > - reg = <0x6a00 0x1000>;
> > - interrupts = <13 IRQ_TYPE_EDGE_RISING>;
> > - resets = <&syscon GEMINI_RESET_TVC>;
> > - clocks = <&syscon GEMINI_CLK_GATE_TVC>,
> > -<&syscon GEMINI_CLK_TVC>;
> > - clock-names = "PCLK", "TVE";
> > -
> > - port@0 {
> > - reg = <0>;
> > - display_out: endpoint {
> > - remote-endpoint = <&panel_in>;
> > - };
> > - };
> > -};
> > diff --git a/Documentation/devicetree/bindings/display/faraday,tve200.yaml
> > b/Documentation/devicetree/bindings/display/faraday,tve200.yaml
> > new file mode 100644
> > index ..3ab51e7e72af
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/faraday,tve200.yaml
> > @@ -0,0 +1,92 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/faraday,tve200.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Faraday TV Encoder TVE200
> > +
> > +maintainers:
> > + - Linus Walleij
> > +
> > +properties:
> > + compatible:
> > +oneOf:
> > + - const: faraday,tve200
> > + - items:
> > + - const: cortina,gemini-tvc
> > + - const: faraday,tve200
> > +
> > + reg:
> > +minItems: 1
>
> maxItems: 1
>
> They evaluate the same, but maxItems seems a bit more logical.
>
> > +
> > + interrupts:
> > +minItems: 1
> > +
> > + clock-names:
> > +items:
> > + - const: PCLK
> > + - const: TVE
> > +
> > + clocks:
> > +minItems: 2
> > +
> > + resets:
> > +minItems: 1
> > +
> > + "#address-cells":
> > +const: 1
> > +
> > + "#size-cells":
> > +const: 0
> > +
> > +patternProperties:
> > + "^port@[0-9]+$":
>
> Should be just 'port' or 'port@0', but really the former is preferred
> when only 1.
>
> Use the graph binding:
>
> $ref: /schemas/graph.yaml#/properties/port
>
I have a problem:
I get the following warning:
/usr/src/linux-next/arch/arm/boot/dts/gemini.dtsi:410.31-423.5: Warning
(graph_child_address): /soc/display-controller@6a00: graph node has single
child node 'port@0', #address-cells/#size-cells are not necessary
also defined at
/usr/src/linux-next/arch/arm/boot/dts/gemini-dlink-dir-685.dts:492.31-501.5
But if I remove them!
/usr/src/linux-next/arch/arm/boot/dts/gemini-dlink-dir-685.dts:496.5-15:
Warning (reg_format): /soc/display-controller@6a00/port@0:reg: property has
invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm/boot/dts/gemini-dlink-dir-685.dt.yaml: Warning (pci_device_reg):
Failed prerequisite 'reg_format'
arch/arm/boot/dts/gemini-dlink-dir-685.dt.yaml: Warning (pci_device_bus_num):
F
Re: [PATCH v3 0/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64
On Thu, Nov 15, 2018 at 09:33:48AM +, Russell King - ARM Linux wrote: > On Thu, Nov 15, 2018 at 10:30:34AM +0100, LABBE Corentin wrote: > > On Wed, Oct 24, 2018 at 09:57:00AM +0100, Russell King - ARM Linux wrote: > > > On Wed, Oct 24, 2018 at 07:35:46AM +, Corentin Labbe wrote: > > > > This patchset adds a new set of functions which are open-coded in lot of > > > > place. > > > > Basicly the pattern is always the same, "read, modify a bit, write" > > > > some driver and the powerpc arch already have thoses pattern them as > > > > functions. (like ahci_sunxi.c or dwmac-meson8b) > > > > > > The advantage of them being open-coded is that it's _obvious_ to the > > > reviewer that there is a read-modify-write going on which, in a multi- > > > threaded environment, may need some locking (so it should trigger a > > > review of the locking around that code.) > > > > > > With it hidden inside a helper which has no locking itself, it becomes > > > much easier to pass over in review, which means that races are much > > > more likely to go unspotted - and that is bad news. > > > > > > > Hello > > > > I understand your fear, but I think the benefit overhaul thoses. > > Furthermore, drivers which I have converted does not need such locking. > > > > If you want I can rename the header to linux/setbits-non-atomic.h for > > making obvious the lack of locking. > > It'd probably be better in the function name - it then doesn't get > "lost" that it's non-atomic when it's included via other headers. > I proposed that way for doing like writeq have do it with io-64-nonatomic-hi-lo.h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64
On Wed, Oct 24, 2018 at 09:57:00AM +0100, Russell King - ARM Linux wrote: > On Wed, Oct 24, 2018 at 07:35:46AM +, Corentin Labbe wrote: > > This patchset adds a new set of functions which are open-coded in lot of > > place. > > Basicly the pattern is always the same, "read, modify a bit, write" > > some driver and the powerpc arch already have thoses pattern them as > > functions. (like ahci_sunxi.c or dwmac-meson8b) > > The advantage of them being open-coded is that it's _obvious_ to the > reviewer that there is a read-modify-write going on which, in a multi- > threaded environment, may need some locking (so it should trigger a > review of the locking around that code.) > > With it hidden inside a helper which has no locking itself, it becomes > much easier to pass over in review, which means that races are much > more likely to go unspotted - and that is bad news. > Hello I understand your fear, but I think the benefit overhaul thoses. Furthermore, drivers which I have converted does not need such locking. If you want I can rename the header to linux/setbits-non-atomic.h for making obvious the lack of locking. Regards ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in linux/setbits.h
On Tue, Sep 25, 2018 at 07:05:00AM +0200, Christophe LEROY wrote: > > > Le 24/09/2018 à 21:04, Corentin Labbe a écrit : > > This patch adds setbits32/clrbits32/clrsetbits32 and > > setbits64/clrbits64/clrsetbits64 in linux/setbits.h header. > > Fix the patch subject and description. > > > > > Signed-off-by: Corentin Labbe > > --- > > include/linux/setbits.h | 88 > > + > > 1 file changed, 88 insertions(+) > > create mode 100644 include/linux/setbits.h > > > > diff --git a/include/linux/setbits.h b/include/linux/setbits.h > > new file mode 100644 > > index ..6e7e257134ae > > --- /dev/null > > +++ b/include/linux/setbits.h > > @@ -0,0 +1,88 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __LINUX_SETBITS_H > > +#define __LINUX_SETBITS_H > > + > > +#include > > + > > +#define __setbits(readfunction, writefunction, addr, set) \ > > + writefunction((readfunction(addr) | (set)), addr) > > You don't need so long names for parameters in a 2 lines macro (See > Linux Kernel Codying style §4 Naming). > > A single line macro would be feasible with only 3 chars names: > > #define __setbits(rfn, wfn, addr, set) wfn((rfn(addr) | (set)), addr) > Thanks I will fix all reported problem. > > +#define __clrbits(readfunction, writefunction, addr, mask) \ > > + writefunction((readfunction(addr) & ~(mask)), addr) > > +#define __clrsetbits(readfunction, writefunction, addr, mask, set) \ > > + writefunction(((readfunction(addr) & ~(mask)) | (set)), addr) > > +#define __setclrbits(readfunction, writefunction, addr, mask, set) \ > > + writefunction(((readfunction(addr) | (set)) & ~(mask)), addr) > > + > > +#ifndef setbits_le32 > > +#define setbits_le32(addr, set) __setbits(readl, writel, addr, set) > > +#endif > > +#ifndef setbits_le32_relaxed > > +#define setbits_le32_relaxed(addr, set) __setbits(readl_relaxed, > > writel_relaxed, \ > > + addr, set) > > +#endif > > + > > +#ifndef clrbits_le32 > > +#define clrbits_le32(addr, mask) __clrbits(readl, writel, addr, mask) > > +#endif > > +#ifndef clrbits_le32_relaxed > > +#define clrbits_le32_relaxed(addr, mask) __clrbits(readl_relaxed, > > writel_relaxed, \ > > + addr, mask) > > +#endif > > + > > +#ifndef clrsetbits_le32 > > +#define clrsetbits_le32(addr, mask, set) __clrsetbits(readl, writel, addr, > > mask, set) > > +#endif > > +#ifndef clrsetbits_le32_relaxed > > +#define clrsetbits_le32_relaxed(addr, mask, set) > > __clrsetbits(readl_relaxed, \ > > + writel_relaxed, \ > > + addr, mask, set) > > +#endif > > + > > +#ifndef setclrbits_le32 > > +#define setclrbits_le32(addr, mask, set) __setclrbits(readl, writel, addr, > > mask, set) > > +#endif > > +#ifndef setclrbits_le32_relaxed > > +#define setclrbits_le32_relaxed(addr, mask, set) > > __setclrbits(readl_relaxed, \ > > + writel_relaxed, \ > > + addr, mask, set) > > +#endif > > + > > +/* We cannot use CONFIG_64BIT as some x86 drivers use non-atomicwriteq() */ > > +#if defined(writeq) && defined(readq) > > Take care. At least Alpha Arch defines it as a static inline without > redefining it as a #define. (see arch/alpha/kernel/io.c) In fact, it does in arch/alpha/include/asm/io.h along with a gentle comment. But fixing their comment will be another interesting patch serie. Regards Corentin Labbe ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/7] powerpc: rename setbits32/clrbits32 to setbits32_be/clrbits32_be
On Tue, Sep 25, 2018 at 06:56:23AM +0200, Christophe LEROY wrote:
> Fix the patch title.
>
>
> Le 24/09/2018 à 21:04, Corentin Labbe a écrit :
> > Since setbits32/clrbits32 work on be32, it's better to remove ambiguity on
> > the used data type.
> >
> > Signed-off-by: Corentin Labbe
> > ---
> > arch/powerpc/include/asm/fsl_lbc.h | 2 +-
> > arch/powerpc/include/asm/io.h| 5 +-
> > arch/powerpc/platforms/44x/canyonlands.c | 4 +-
> > arch/powerpc/platforms/4xx/gpio.c| 28 -
> > arch/powerpc/platforms/512x/pdm360ng.c | 6 +-
> > arch/powerpc/platforms/52xx/mpc52xx_common.c | 6 +-
> > arch/powerpc/platforms/52xx/mpc52xx_gpt.c| 10 ++--
> > arch/powerpc/platforms/82xx/ep8248e.c| 2 +-
> > arch/powerpc/platforms/82xx/km82xx.c | 6 +-
> > arch/powerpc/platforms/82xx/mpc8272_ads.c| 10 ++--
> > arch/powerpc/platforms/82xx/pq2.c| 2 +-
> > arch/powerpc/platforms/82xx/pq2ads-pci-pic.c | 4 +-
> > arch/powerpc/platforms/82xx/pq2fads.c| 10 ++--
> > arch/powerpc/platforms/83xx/km83xx.c | 6 +-
> > arch/powerpc/platforms/83xx/mpc836x_mds.c| 2 +-
> > arch/powerpc/platforms/85xx/mpc85xx_mds.c| 2 +-
> > arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 4 +-
> > arch/powerpc/platforms/85xx/mpc85xx_rdb.c| 2 +-
> > arch/powerpc/platforms/85xx/p1022_ds.c | 4 +-
> > arch/powerpc/platforms/85xx/p1022_rdk.c | 4 +-
> > arch/powerpc/platforms/85xx/t1042rdb_diu.c | 4 +-
> > arch/powerpc/platforms/85xx/twr_p102x.c | 2 +-
> > arch/powerpc/platforms/86xx/mpc8610_hpcd.c | 4 +-
> > arch/powerpc/platforms/8xx/adder875.c| 2 +-
> > arch/powerpc/platforms/8xx/m8xx_setup.c | 4 +-
> > arch/powerpc/platforms/8xx/mpc86xads_setup.c | 4 +-
> > arch/powerpc/platforms/8xx/mpc885ads_setup.c | 28 -
> > arch/powerpc/platforms/embedded6xx/flipper-pic.c | 6 +-
> > arch/powerpc/platforms/embedded6xx/hlwd-pic.c| 8 +--
> > arch/powerpc/platforms/embedded6xx/wii.c | 10 ++--
> > arch/powerpc/sysdev/cpm1.c | 26 -
> > arch/powerpc/sysdev/cpm2.c | 16 ++---
> > arch/powerpc/sysdev/cpm_common.c | 4 +-
> > arch/powerpc/sysdev/fsl_85xx_l2ctlr.c| 8 +--
> > arch/powerpc/sysdev/fsl_lbc.c| 2 +-
> > arch/powerpc/sysdev/fsl_pci.c| 8 +--
> > arch/powerpc/sysdev/fsl_pmc.c| 2 +-
> > arch/powerpc/sysdev/fsl_rcpm.c | 74
> >
> > arch/powerpc/sysdev/fsl_rio.c| 4 +-
> > arch/powerpc/sysdev/fsl_rmu.c| 8 +--
> > arch/powerpc/sysdev/mpic_timer.c | 12 ++--
> > 41 files changed, 178 insertions(+), 177 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/fsl_lbc.h
> > b/arch/powerpc/include/asm/fsl_lbc.h
> > index c7240a024b96..4d6a56b48a28 100644
> > --- a/arch/powerpc/include/asm/fsl_lbc.h
> > +++ b/arch/powerpc/include/asm/fsl_lbc.h
> > @@ -276,7 +276,7 @@ static inline void fsl_upm_start_pattern(struct fsl_upm
> > *upm, u8 pat_offset)
> >*/
> > static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
> > {
> > - clrbits32(upm->mxmr, MxMR_OP_RP);
> > + clrbits_be32(upm->mxmr, MxMR_OP_RP);
> >
> > while (in_be32(upm->mxmr) & MxMR_OP_RP)
> > cpu_relax();
> > diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> > index e0331e754568..57486a1b9992 100644
> > --- a/arch/powerpc/include/asm/io.h
> > +++ b/arch/powerpc/include/asm/io.h
> > @@ -873,8 +873,8 @@ static inline void * bus_to_virt(unsigned long address)
> > #endif /* CONFIG_PPC32 */
> >
> > /* access ports */
> > -#define setbits32(_addr, _v) out_be32((_addr), in_be32(_addr) | (_v))
> > -#define clrbits32(_addr, _v) out_be32((_addr), in_be32(_addr) & ~(_v))
> > +#define setbits_be32(_addr, _v) out_be32((_addr), in_be32(_addr) | (_v))
> > +#define clrbits_be32(_addr, _v) out_be32((_addr), in_be32(_addr) & ~(_v))
> >
> > #define setbits16(_addr, _v) out_be16((_addr), in_be16(_addr) | (_v))
> > #define clrbits16(_addr, _v) out_be16((_addr), in_be16(_addr) & ~(_v))
> > @@ -904,6 +904,7 @@ static inline void * bus_to_virt(unsigned long address)
> > #define clrsetbits_le16(addr, clear, set) clrsetbits(le16, addr, clear,
> > set)
> >
> > #define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)
> > +#define clrsetbits_be32(addr, clear, set) clrsetbits(be32, addr, clear,
> > set)
>
> This one already exists a few lines above.
>
> >
> > #endif /* __KERNEL__ */
> >
> > diff --git a/arch/powerpc/platforms/44x/canyonlands.c
> > b/arch/powerpc/platforms/44x/canyonlands.c
> > index 157f4ce46386..6aeb4ca64d09 100644
[PATCH v2] gpu: ipu-v3: fix a possible NULL dereference
of_match_device could return NULL, and so cause a NULL pointer
dereference later.
For fixing this problem, we use of_device_get_match_data(), this will
simplify the code a little by using a standard function for
getting the match data.
Testing the return value of of_device_get_match_data is also necessary
for avoiding a second NULL deref later on devtype.
Signed-off-by: LABBE Corentin
---
Changes in v2:
- Add a test on devtype for avoiding a second NULL deref.
drivers/gpu/ipu-v3/ipu-common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index 99dcacf..8072470 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -1207,15 +1207,15 @@ EXPORT_SYMBOL_GPL(ipu_dump);
static int ipu_probe(struct platform_device *pdev)
{
- const struct of_device_id *of_id =
- of_match_device(imx_ipu_dt_ids, &pdev->dev);
struct ipu_soc *ipu;
struct resource *res;
unsigned long ipu_base;
int i, ret, irq_sync, irq_err;
const struct ipu_devtype *devtype;
- devtype = of_id->data;
+ devtype = of_device_get_match_data(&pdev->dev);
+ if (!devtype)
+ return -EINVAL;
irq_sync = platform_get_irq(pdev, 0);
irq_err = platform_get_irq(pdev, 1);
--
2.7.3
[PATCH] gpu: ipu-v3: fix a possible NULL dereference
of_match_device could return NULL, and so cause a NULL pointer
dereference later.
For fixing this problem, we use of_device_get_match_data(), this will
simplify the code a little by using a standard function for
getting the match data.
Signed-off-by: LABBE Corentin
---
drivers/gpu/ipu-v3/ipu-common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index 99dcacf..05a9cc6 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -1207,15 +1207,13 @@ EXPORT_SYMBOL_GPL(ipu_dump);
static int ipu_probe(struct platform_device *pdev)
{
- const struct of_device_id *of_id =
- of_match_device(imx_ipu_dt_ids, &pdev->dev);
struct ipu_soc *ipu;
struct resource *res;
unsigned long ipu_base;
int i, ret, irq_sync, irq_err;
const struct ipu_devtype *devtype;
- devtype = of_id->data;
+ devtype = of_device_get_match_data(&pdev->dev);
irq_sync = platform_get_irq(pdev, 0);
irq_err = platform_get_irq(pdev, 1);
--
2.7.3
[PATCH] drm: modes: add missing [drm] to message printing
The warning message in drm_mode_parse_command_line_for_connector miss
the [drm] at beginning.
This patch add it and take the opportunity to convert
printk(KERN_WARNING to pr_warn()
Signed-off-by: LABBE Corentin
---
drivers/gpu/drm/drm_modes.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 20775c0..f7448a5 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1371,8 +1371,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
}
done:
if (i >= 0) {
- printk(KERN_WARNING
- "parse error at position %i in video mode '%s'\n",
+ pr_warn("[drm] parse error at position %i in video mode '%s'\n",
i, name);
mode->specified = false;
return false;
--
2.4.10
[PATCH] drm: modes: Revert cc344980c767 "replace simple_strtoul by kstrtouint"
My latest commit introduce some case where a valid mode, could be
rejected.
simple_strtox functions stop at first non-digit character, but kstrtox
not.
So args like "video=HDMI-A-1:720x480-16 at 60" will be reject when checking
16 at .
Discussions about this change comes to the conclusion that the best
solution is to revert my commit cc344980c76748e57c9c03100c2a14d36ab00334.
Signed-off-by: LABBE Corentin
---
drivers/gpu/drm/drm_modes.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index db36af7..3da1434 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1230,7 +1230,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
bool yres_specified = false, cvt = false, rb = false;
bool interlace = false, margins = false, was_digit = false;
- int i, err;
+ int i;
enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
#ifdef CONFIG_FB
@@ -1250,9 +1250,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
case '@':
if (!refresh_specified && !bpp_specified &&
!yres_specified && !cvt && !rb && was_digit) {
- err = kstrtouint(&name[i + 1], 10, &refresh);
- if (err)
- return false;
+ refresh = simple_strtol(&name[i+1], NULL, 10);
refresh_specified = true;
was_digit = false;
} else
@@ -1261,9 +1259,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
case '-':
if (!bpp_specified && !yres_specified && !cvt &&
!rb && was_digit) {
- err = kstrtouint(&name[i + 1], 10, &bpp);
- if (err)
- return false;
+ bpp = simple_strtol(&name[i+1], NULL, 10);
bpp_specified = true;
was_digit = false;
} else
@@ -1271,9 +1267,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
break;
case 'x':
if (!yres_specified && was_digit) {
- err = kstrtouint(&name[i + 1], 10, &yres);
- if (err)
- return false;
+ yres = simple_strtol(&name[i+1], NULL, 10);
yres_specified = true;
was_digit = false;
} else
@@ -1496,4 +1490,4 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
out:
return ret;
-}
+}
\ No newline at end of file
--
2.4.10
[PATCH v2 1/1] drm: modes: fix DRM modes analysis regression
My latest commit introduce some case where a valid mode, could be
rejected.
simple_strtox functions stop at first non-digit character, but kstrtox not.
So args like "video=HDMI-A-1:720x480-16 at 60" will be reject when checking 16
at .
The proper solution is to store digits in a specific buffer.
Fixes: 52157a4ca396 ("drm: modes: replace simple_strtoul by kstrtouint")
Reported-by: Kuninori Morimoto
Signed-off-by: LABBE Corentin
---
drivers/gpu/drm/drm_modes.c | 28 +---
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index bde9b29..da1e80d 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1225,13 +1225,14 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
struct drm_cmdline_mode *mode)
{
const char *name;
- unsigned int namelen;
+ unsigned int namelen, digit_i;
bool res_specified = false, bpp_specified = false, refresh_specified =
false;
unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
bool yres_specified = false, cvt = false, rb = false;
bool interlace = false, margins = false, was_digit = false;
int i, err;
enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
+ char *digits;
#ifdef CONFIG_FB
if (!mode_option)
@@ -1245,42 +1246,53 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
name = mode_option;
namelen = strlen(name);
+
+ digits = kzalloc(namelen, GFP_KERNEL);
+ if (!digits)
+ return false;
+ /* The last character must be the last 0 */
+ digit_i = namelen;
+
for (i = namelen-1; i >= 0; i--) {
switch (name[i]) {
case '@':
if (!refresh_specified && !bpp_specified &&
!yres_specified && !cvt && !rb && was_digit) {
- err = kstrtouint(&name[i + 1], 10, &refresh);
+ err = kstrtouint(&digits[digit_i], 10,
&refresh);
if (err)
- return false;
+ goto done;
refresh_specified = true;
was_digit = false;
+ digit_i = namelen;
} else
goto done;
break;
case '-':
if (!bpp_specified && !yres_specified && !cvt &&
!rb && was_digit) {
- err = kstrtouint(&name[i + 1], 10, &bpp);
+ err = kstrtouint(&digits[digit_i], 10, &bpp);
if (err)
- return false;
+ goto done;
bpp_specified = true;
was_digit = false;
+ digit_i = namelen;
} else
goto done;
break;
case 'x':
if (!yres_specified && was_digit) {
- err = kstrtouint(&name[i + 1], 10, &yres);
+ err = kstrtouint(&digits[digit_i], 10, &yres);
if (err)
- return false;
+ goto done;
yres_specified = true;
was_digit = false;
+ digit_i = namelen;
} else
goto done;
break;
case '0' ... '9':
was_digit = true;
+ digits[--digit_i] = name[i];
break;
case 'M':
if (yres_specified || cvt || was_digit)
@@ -1349,6 +1361,7 @@ done:
"parse error at position %i in video mode '%s'\n",
i, name);
mode->specified = false;
+ kfree(digits);
return false;
}
@@ -1373,6 +1386,7 @@ done:
mode->margins = margins;
mode->force = force;
+ kfree(digits);
return true;
}
EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector);
--
2.4.10
[PATCH v2 0/1] drm: modes: fix DRM modes analysis regression
Hello Changes since v1 - Fix a memory leak when returning after an error of kstrtox LABBE Corentin (1): drm: modes: fix DRM modes analysis regression drivers/gpu/drm/drm_modes.c | 28 +--- 1 file changed, 21 insertions(+), 7 deletions(-) -- 2.4.10
[PATCH] drm: modes: replace simple_strtoul by kstrtouint
The simple_strtoul function is marked as obsolete.
This patch replace it by kstrtouint.
Signed-off-by: LABBE Corentin
---
drivers/gpu/drm/drm_modes.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index cd74a09..bde9b29 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1230,7 +1230,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
bool yres_specified = false, cvt = false, rb = false;
bool interlace = false, margins = false, was_digit = false;
- int i;
+ int i, err;
enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
#ifdef CONFIG_FB
@@ -1250,7 +1250,9 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
case '@':
if (!refresh_specified && !bpp_specified &&
!yres_specified && !cvt && !rb && was_digit) {
- refresh = simple_strtol(&name[i+1], NULL, 10);
+ err = kstrtouint(&name[i + 1], 10, &refresh);
+ if (err)
+ return false;
refresh_specified = true;
was_digit = false;
} else
@@ -1259,7 +1261,9 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
case '-':
if (!bpp_specified && !yres_specified && !cvt &&
!rb && was_digit) {
- bpp = simple_strtol(&name[i+1], NULL, 10);
+ err = kstrtouint(&name[i + 1], 10, &bpp);
+ if (err)
+ return false;
bpp_specified = true;
was_digit = false;
} else
@@ -1267,7 +1271,9 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
break;
case 'x':
if (!yres_specified && was_digit) {
- yres = simple_strtol(&name[i+1], NULL, 10);
+ err = kstrtouint(&name[i + 1], 10, &yres);
+ if (err)
+ return false;
yres_specified = true;
was_digit = false;
} else
@@ -1491,4 +1497,4 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
out:
return ret;
-}
\ No newline at end of file
+}
--
2.4.10
