Re: [PATCH 09/22] gpio: virtio: drop owner assignment
On Fri, Mar 29, 2024 at 12:35 PM Krzysztof Kozlowski
wrote:
>
> On 29/03/2024 11:27, Bartosz Golaszewski wrote:
> > On Wed, Mar 27, 2024 at 1:45 PM Krzysztof Kozlowski
> > wrote:
> >>
> >> virtio core already sets the .owner, so driver does not need to.
> >>
> >> Signed-off-by: Krzysztof Kozlowski
> >>
> >> ---
> >>
> >> Depends on the first patch.
> >> ---
> >> drivers/gpio/gpio-virtio.c | 1 -
> >> 1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> >> index fcc5e8c08973..9fae8e396c58 100644
> >> --- a/drivers/gpio/gpio-virtio.c
> >> +++ b/drivers/gpio/gpio-virtio.c
> >> @@ -653,7 +653,6 @@ static struct virtio_driver virtio_gpio_driver = {
> >> .remove = virtio_gpio_remove,
> >> .driver = {
> >> .name = KBUILD_MODNAME,
> >> - .owner = THIS_MODULE,
> >> },
> >> };
> >> module_virtio_driver(virtio_gpio_driver);
> >>
> >> --
> >> 2.34.1
> >>
> >
> > Applied, thanks!
>
> I expressed dependency in two places: cover letter and this patch.
> Please drop it, because without dependency this won't work. Patch could
> go with the dependency and with your ack or next cycle.
>
> Best regards,
> Krzysztof
>
Dropped, and:
Acked-by: Bartosz Golaszewski
Re: [PATCH 09/22] gpio: virtio: drop owner assignment
On Wed, Mar 27, 2024 at 1:45 PM Krzysztof Kozlowski
wrote:
>
> virtio core already sets the .owner, so driver does not need to.
>
> Signed-off-by: Krzysztof Kozlowski
>
> ---
>
> Depends on the first patch.
> ---
> drivers/gpio/gpio-virtio.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> index fcc5e8c08973..9fae8e396c58 100644
> --- a/drivers/gpio/gpio-virtio.c
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -653,7 +653,6 @@ static struct virtio_driver virtio_gpio_driver = {
> .remove = virtio_gpio_remove,
> .driver = {
> .name = KBUILD_MODNAME,
> - .owner = THIS_MODULE,
> },
> };
> module_virtio_driver(virtio_gpio_driver);
>
> --
> 2.34.1
>
Applied, thanks!
Bart
Re: [PATCH 1/2] devm-helpers: Add resource managed version of mutex init
On Thu, Feb 22, 2024 at 3:58 PM Marek Behún wrote: > > A few drivers are doing resource-managed mutex initialization by > implementing ad-hoc one-liner mutex dropping functions and using them > with devm_add_action_or_reset(). Help drivers avoid these repeated > one-liners by adding managed version of mutex initialization. > > Use the new function devm_mutex_init() in the following drivers: > drivers/gpio/gpio-pisosr.c > drivers/gpio/gpio-sim.c Yes, please! For GPIO part: Acked-by: Bartosz Golaszewski
Re: [PATCH v4] backlight: gpio_backlight: Drop output GPIO direction check for initial power state
On Fri, Jul 21, 2023 at 11:29 AM Ying Liu wrote:
>
> If GPIO pin is in input state but backlight is currently off due to
> default pull downs, then initial power state is set to FB_BLANK_UNBLANK
> in DT boot mode with phandle link and the backlight is effectively
> turned on in gpio_backlight_probe(), which is undesirable according to
> patch description of commit ec665b756e6f ("backlight: gpio-backlight:
> Correct initial power state handling").
>
> Quote:
> ---8<---
> If in DT boot we have phandle link then leave the GPIO in a state which the
> bootloader left it and let the user of the backlight to configure it further.
> ---8<---
>
> So, let's drop output GPIO direction check and only check GPIO value to set
> the initial power state.
>
> Fixes: 706dc68102bc ("backlight: gpio: Explicitly set the direction of the
> GPIO")
> Signed-off-by: Liu Ying
> ---
> v3->v4:
> * Capitalize words 'gpio' in patch description. (Andy)
> * Capitalize word 'gpio' in patch title.
> * Quote appropriately in patch description. (Andy)
>
> v2->v3:
> * Add Fixes tag. (Daniel)
>
> v1->v2:
> * Improve patch description. (Daniel, Bartosz, Andy)
>
> drivers/video/backlight/gpio_backlight.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/gpio_backlight.c
> b/drivers/video/backlight/gpio_backlight.c
> index 6f78d928f054..38c46936fdcd 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -87,8 +87,7 @@ static int gpio_backlight_probe(struct platform_device
> *pdev)
> /* Not booted with device tree or no phandle link to the node
> */
> bl->props.power = def_value ? FB_BLANK_UNBLANK
> : FB_BLANK_POWERDOWN;
> - else if (gpiod_get_direction(gbl->gpiod) == 0 &&
> -gpiod_get_value_cansleep(gbl->gpiod) == 0)
> + else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> bl->props.power = FB_BLANK_POWERDOWN;
> else
> bl->props.power = FB_BLANK_UNBLANK;
> --
> 2.37.1
>
Acked-by: Bartosz Golaszewski
Re: [PATCH] backlight: gpio_backlight: Drop output gpio direction check for initial power state
On Thu, Jul 20, 2023 at 3:10 PM Daniel Thompson
wrote:
>
> On Thu, Jul 20, 2023 at 02:56:32PM +0200, Bartosz Golaszewski wrote:
> > On Thu, Jul 20, 2023 at 1:27 PM Daniel Thompson
> > wrote:
> > >
> > > On Thu, Jul 20, 2023 at 06:06:27AM +, Ying Liu wrote:
> > > > Bootloader may leave gpio direction as input and gpio value as logical
> > > > low.
> > > > It hints that initial backlight power state should be FB_BLANK_POWERDOWN
> > > > since the gpio value is literally logical low.
> > >
> > > To be honest this probably "hints" that the bootloader simply didn't
> > > consider the backlight at all :-) . I'd rather the patch description
> > > focus on what circumstances lead to the current code making a bad
> > > decision. More like:
> > >
> > > If the GPIO pin is in the input state but the backlight is currently
> > > off due to default pull downs then ...
> > >
> > > > So, let's drop output gpio
> > > > direction check and only check gpio value to set the initial power
> > > > state.
> > >
> > > This check was specifically added by Bartosz so I'd be interested in his
> > > opinion of this change (especially since he is now a GPIO maintainer)!
> > >
> > > What motivates (or motivated) the need to check the direction rather
> > > than just read that current logic level on the pin?
> > >
> > >
> > > Daniel.
> > > [I'm done but since Bartosz and Linus were not on copy of the original
> > > thread I've left the rest of the patch below as a convenience ;-) ]
> > >
> >
> > This was done in commit: 706dc68102bc ("backlight: gpio: Explicitly
> > set the direction of the GPIO").
> >
> > Let me quote myself from it:
> > --
> > The GPIO backlight driver currently requests the line 'as is', without
> > actively setting its direction. This can lead to problems: if the line
> > is in input mode by default, we won't be able to drive it later when
> > updating the status and also reading its initial value doesn't make
> > sense for backlight setting.
> > --
>
> You are perhaps quoting the wrong bit here ;-). The currently proposed
> patch leaves the code to put the pin into output mode unmodified. However
> there was an extra line at the bottom of your commit message:
> --
> Also: check the current direction and only read the value if it's output.
> --
Yeah I'm no longer sure why I did this. The commit doesn't look harmful though.
Bart
>
> This was the bit I wanted to check on, since the proposed patch
> literally reverses this!
>
> However...
>
>
> > I agree with Thomas that it's highly unlikely the bootloader "hints"
> > at any specific backlight settings. That being said, the change itself
> > looks correct to me. The other branch of that if will always unblank
> > the backlight if the GPIO is in input mode which may not be desirable.
>
> ... if you're happy the proposed change is OK then I'm happy too!
> I came to the same conclusion after reviewing the GPIO code this morning,
> however I copied you in because I was worried I might have overlooked
> something.
>
>
> > I don't see any obvious problem with this change, just make sure the
> > commit message makes more sense.
>
> Agreed.
>
>
> Daniel.
Re: [PATCH] backlight: gpio_backlight: Drop output gpio direction check for initial power state
On Thu, Jul 20, 2023 at 1:27 PM Daniel Thompson
wrote:
>
> On Thu, Jul 20, 2023 at 06:06:27AM +, Ying Liu wrote:
> > Bootloader may leave gpio direction as input and gpio value as logical low.
> > It hints that initial backlight power state should be FB_BLANK_POWERDOWN
> > since the gpio value is literally logical low.
>
> To be honest this probably "hints" that the bootloader simply didn't
> consider the backlight at all :-) . I'd rather the patch description
> focus on what circumstances lead to the current code making a bad
> decision. More like:
>
> If the GPIO pin is in the input state but the backlight is currently
> off due to default pull downs then ...
>
> > So, let's drop output gpio
> > direction check and only check gpio value to set the initial power state.
>
> This check was specifically added by Bartosz so I'd be interested in his
> opinion of this change (especially since he is now a GPIO maintainer)!
>
> What motivates (or motivated) the need to check the direction rather
> than just read that current logic level on the pin?
>
>
> Daniel.
> [I'm done but since Bartosz and Linus were not on copy of the original
> thread I've left the rest of the patch below as a convenience ;-) ]
>
This was done in commit: 706dc68102bc ("backlight: gpio: Explicitly
set the direction of the GPIO").
Let me quote myself from it:
--
The GPIO backlight driver currently requests the line 'as is', without
actively setting its direction. This can lead to problems: if the line
is in input mode by default, we won't be able to drive it later when
updating the status and also reading its initial value doesn't make
sense for backlight setting.
--
I agree with Thomas that it's highly unlikely the bootloader "hints"
at any specific backlight settings. That being said, the change itself
looks correct to me. The other branch of that if will always unblank
the backlight if the GPIO is in input mode which may not be desirable.
I don't see any obvious problem with this change, just make sure the
commit message makes more sense.
Bartosz
Re: [PATCH] dt-bindings: Fix SPI and I2C bus node names in examples
On Tue, Feb 28, 2023 at 10:54 PM Rob Herring wrote:
>
> SPI and I2C bus node names are expected to be "spi" or "i2c",
> respectively, with nothing else, a unit-address, or a '-N' index. A
> pattern of 'spi0' or 'i2c0' or similar has crept in. Fix all these
> cases. Mostly scripted with the following commands:
>
> git grep -l '\si2c[0-9] {' Documentation/devicetree/ | xargs sed -i -e
> 's/i2c[0-9] {/i2c {/'
> git grep -l '\sspi[0-9] {' Documentation/devicetree/ | xargs sed -i -e
> 's/spi[0-9] {/spi {/'
>
> With this, a few errors in examples were exposed and fixed.
>
> Signed-off-by: Rob Herring
> ---
> Cc: Miguel Ojeda
> Cc: Krzysztof Kozlowski
> Cc: Benson Leung
> Cc: Guenter Roeck
> Cc: Stephen Boyd
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Thierry Reding
> Cc: Sam Ravnborg
> Cc: MyungJoo Ham
> Cc: Chanwoo Choi
> Cc: Linus Walleij
> Cc: Bartosz Golaszewski
> Cc: Pavel Machek
> Cc: Lee Jones
> Cc: Mauro Carvalho Chehab
> Cc: "David S. Miller"
> Cc: Eric Dumazet
> Cc: Jakub Kicinski
> Cc: Paolo Abeni
> Cc: Wolfgang Grandegger
> Cc: Kalle Valo
> Cc: Sebastian Reichel
> Cc: Mark Brown
> Cc: Greg Kroah-Hartman
> Cc: linux-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-g...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-l...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: net...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-wirel...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: alsa-de...@alsa-project.org
> Cc: linux-...@vger.kernel.org
> ---
> .../bindings/auxdisplay/holtek,ht16k33.yaml | 2 +-
> .../bindings/chrome/google,cros-ec-typec.yaml | 2 +-
> .../chrome/google,cros-kbd-led-backlight.yaml | 2 +-
> .../devicetree/bindings/clock/ti,lmk04832.yaml| 2 +-
> .../bindings/display/bridge/analogix,anx7625.yaml | 2 +-
> .../bindings/display/bridge/anx6345.yaml | 2 +-
> .../bindings/display/bridge/lontium,lt8912b.yaml | 2 +-
> .../bindings/display/bridge/nxp,ptn3460.yaml | 2 +-
> .../bindings/display/bridge/ps8640.yaml | 2 +-
> .../bindings/display/bridge/sil,sii9234.yaml | 2 +-
> .../bindings/display/bridge/ti,dlpc3433.yaml | 2 +-
> .../bindings/display/bridge/toshiba,tc358762.yaml | 2 +-
> .../bindings/display/bridge/toshiba,tc358768.yaml | 2 +-
> .../bindings/display/panel/nec,nl8048hl11.yaml| 2 +-
> .../bindings/display/solomon,ssd1307fb.yaml | 4 ++--
> .../devicetree/bindings/eeprom/at25.yaml | 2 +-
> .../bindings/extcon/extcon-usbc-cros-ec.yaml | 2 +-
> .../bindings/extcon/extcon-usbc-tusb320.yaml | 2 +-
> .../devicetree/bindings/gpio/gpio-pca9570.yaml| 2 +-
> .../devicetree/bindings/gpio/gpio-pca95xx.yaml| 8
Acked-by: Bartosz Golaszewski
Re: [PATCH] dt-bindings: Add missing (unevaluated|additional)Properties on child node schemas
On Wed, Jan 25, 2023 at 12:00 AM Rob Herring wrote: > > Just as unevaluatedProperties or additionalProperties are required at > the top level of schemas, they should (and will) also be required for > child node schemas. That ensures only documented properties are > present. > > Add unevaluatedProperties or additionalProperties as appropriate, and > then add any missing properties flagged by the addition. > > Signed-off-by: Rob Herring > --- [...] > .../devicetree/bindings/gpio/x-powers,axp209-gpio.yaml | 1 + [...] > diff --git a/Documentation/devicetree/bindings/gpio/x-powers,axp209-gpio.yaml > b/Documentation/devicetree/bindings/gpio/x-powers,axp209-gpio.yaml > index 7f26f6b1eea1..31906c253940 100644 > --- a/Documentation/devicetree/bindings/gpio/x-powers,axp209-gpio.yaml > +++ b/Documentation/devicetree/bindings/gpio/x-powers,axp209-gpio.yaml > @@ -35,6 +35,7 @@ properties: > patternProperties: >"^.*-pins?$": > $ref: /schemas/pinctrl/pinmux-node.yaml# > + additionalProperties: false > > properties: >pins: Acked-by: Bartosz Golaszewski [...]
Re: [PATCH] dt-bindings: Fix properties without any type
On Thu, May 19, 2022 at 11:14 PM Rob Herring wrote: > > Now that the schema tools can extract type information for all > properties (in order to decode dtb files), finding properties missing > any type definition is fairly trivial though not yet automated. > > Fix the various property schemas which are missing a type. Most of these > tend to be device specific properties which don't have a vendor prefix. > A vendor prefix is how we normally ensure a type is defined. > > Signed-off-by: Rob Herring > --- > .../arm/hisilicon/controller/hip04-bootwrapper.yaml | 5 +++-- > .../bindings/display/bridge/toshiba,tc358768.yaml | 1 + > .../devicetree/bindings/display/panel/panel-timing.yaml | 5 + > .../bindings/display/panel/raydium,rm67191.yaml | 1 + > .../bindings/display/panel/samsung,s6e8aa0.yaml | 1 + > .../devicetree/bindings/gpio/fairchild,74hc595.yaml | 1 + > .../devicetree/bindings/input/google,cros-ec-keyb.yaml| 1 + > .../devicetree/bindings/input/matrix-keymap.yaml | 4 > Documentation/devicetree/bindings/media/i2c/adv7604.yaml | 3 ++- > Documentation/devicetree/bindings/mux/reg-mux.yaml| 8 ++-- > Documentation/devicetree/bindings/net/cdns,macb.yaml | 1 + > Documentation/devicetree/bindings/net/ingenic,mac.yaml| 1 + > .../devicetree/bindings/net/ti,davinci-mdio.yaml | 1 + > .../devicetree/bindings/net/wireless/ti,wlcore.yaml | 2 ++ > .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 6 -- > Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 2 ++ > .../devicetree/bindings/pinctrl/canaan,k210-fpioa.yaml| 2 ++ > Documentation/devicetree/bindings/power/avs/qcom,cpr.yaml | 1 + > .../devicetree/bindings/power/supply/battery.yaml | 7 ++- > .../devicetree/bindings/power/supply/charger-manager.yaml | 1 + > Documentation/devicetree/bindings/rng/st,stm32-rng.yaml | 1 + > Documentation/devicetree/bindings/serial/8250.yaml| 1 + > .../devicetree/bindings/sound/audio-graph-card2.yaml | 3 +++ > .../devicetree/bindings/sound/imx-audio-hdmi.yaml | 3 +++ > Documentation/devicetree/bindings/usb/smsc,usb3503.yaml | 1 + > 25 files changed, 55 insertions(+), 8 deletions(-) > For GPIO: Acked-by: Bartosz Golaszewski
Re: [PATCH] dt-bindings: Add missing array size constraints
On Tue, Jan 5, 2021 at 12:03 AM Rob Herring wrote: > > DT properties which can have multiple entries need to specify what the > entries are and define how many entries there can be. In the case of > only a single entry, just 'maxItems: 1' is sufficient. > > Add the missing entry constraints. These were found with a modified > meta-schema. Unfortunately, there are a few cases where the size > constraints are not defined such as common bindings, so the meta-schema > can't be part of the normal checks. > [snip!] > .../bindings/gpio/gpio-pca95xx.yaml | 1 + [snip!] > diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml > b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml > index f5ee23c2df60..57cdcfd4ff3c 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml > +++ b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml > @@ -81,6 +81,7 @@ properties: > const: 2 > >reset-gpios: > +maxItems: 1 > description: >GPIO specification for the RESET input. This is an active low signal to > the PCA953x. Not valid for Maxim MAX732x devices. Acked-by: Bartosz Golaszewski ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 6/9] edac: ghes: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Acked-by: Borislav Petkov
---
drivers/edac/ghes_edac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index a918ca93e4f7..6d1ddecbf0da 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -207,8 +207,8 @@ static void enumerate_dimms(const struct dmi_header *dh,
void *arg)
if (!hw->num_dimms || !(hw->num_dimms % 16)) {
struct dimm_info *new;
- new = krealloc(hw->dimms, (hw->num_dimms + 16) * sizeof(struct
dimm_info),
- GFP_KERNEL);
+ new = krealloc_array(hw->dimms, hw->num_dimms + 16,
+sizeof(struct dimm_info), GFP_KERNEL);
if (!new) {
WARN_ON_ONCE(1);
return;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 8/9] hwtracing: intel: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
drivers/hwtracing/intel_th/msu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 3a77551fb4fc..7d95242db900 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -2002,7 +2002,7 @@ nr_pages_store(struct device *dev, struct
device_attribute *attr,
}
nr_wins++;
- rewin = krealloc(win, sizeof(*win) * nr_wins, GFP_KERNEL);
+ rewin = krealloc_array(win, nr_wins, sizeof(*win), GFP_KERNEL);
if (!rewin) {
kfree(win);
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 7/9] drm: atomic: use krealloc_array()
From: Bartosz Golaszewski Use the helper that checks for overflows internally instead of manually calculating the size of the new array. Signed-off-by: Bartosz Golaszewski Acked-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 58527f151984..09ad6a2ec17b 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -960,7 +960,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, struct __drm_connnectors_state *c; int alloc = max(index + 1, config->num_connector); - c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL); + c = krealloc_array(state->connectors, alloc, + sizeof(*state->connectors), GFP_KERNEL); if (!c) return ERR_PTR(-ENOMEM); -- 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 5/9] pinctrl: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
---
drivers/pinctrl/pinctrl-utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-utils.c b/drivers/pinctrl/pinctrl-utils.c
index f2bcbf62c03d..93df0d4c0a24 100644
--- a/drivers/pinctrl/pinctrl-utils.c
+++ b/drivers/pinctrl/pinctrl-utils.c
@@ -39,7 +39,7 @@ int pinctrl_utils_reserve_map(struct pinctrl_dev *pctldev,
if (old_num >= new_num)
return 0;
- new_map = krealloc(*map, sizeof(*new_map) * new_num, GFP_KERNEL);
+ new_map = krealloc_array(*map, new_num, sizeof(*new_map), GFP_KERNEL);
if (!new_map) {
dev_err(pctldev->dev, "krealloc(map) failed\n");
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 9/9] dma-buf: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Acked-by: Christian König
---
drivers/dma-buf/sync_file.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 5a5a1da01a00..20d9bddbb985 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -270,8 +270,7 @@ static struct sync_file *sync_file_merge(const char *name,
struct sync_file *a,
fences[i++] = dma_fence_get(a_fences[0]);
if (num_fences > i) {
- nfences = krealloc(fences, i * sizeof(*fences),
- GFP_KERNEL);
+ nfences = krealloc_array(fences, i, sizeof(*fences),
GFP_KERNEL);
if (!nfences)
goto err;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/9] vhost: vringh: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Acked-by: Michael S. Tsirkin
---
drivers/vhost/vringh.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 8bd8b403f087..08a0e1c842df 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -198,7 +198,8 @@ static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
if (flag)
- new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
+ new = krealloc_array(iov->iov, new_num,
+sizeof(struct iovec), gfp);
else {
new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
if (new) {
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/9] mm: slab: provide krealloc_array()
From: Bartosz Golaszewski
When allocating an array of elements, users should check for
multiplication overflow or preferably use one of the provided helpers
like: kmalloc_array().
There's no krealloc_array() counterpart but there are many users who use
regular krealloc() to reallocate arrays. Let's provide an actual
krealloc_array() implementation.
While at it: add some documentation regarding krealloc.
Signed-off-by: Bartosz Golaszewski
Acked-by: Vlastimil Babka
---
Documentation/core-api/memory-allocation.rst | 4
include/linux/slab.h | 18 ++
2 files changed, 22 insertions(+)
diff --git a/Documentation/core-api/memory-allocation.rst
b/Documentation/core-api/memory-allocation.rst
index 4446a1ac36cc..5954ddf6ee13 100644
--- a/Documentation/core-api/memory-allocation.rst
+++ b/Documentation/core-api/memory-allocation.rst
@@ -147,6 +147,10 @@ The address of a chunk allocated with `kmalloc` is aligned
to at least
ARCH_KMALLOC_MINALIGN bytes. For sizes which are a power of two, the
alignment is also guaranteed to be at least the respective size.
+Chunks allocated with kmalloc() can be resized with krealloc(). Similarly
+to kmalloc_array(): a helper for resizing arrays is provided in the form of
+krealloc_array().
+
For large allocations you can use vmalloc() and vzalloc(), or directly
request pages from the page allocator. The memory allocated by `vmalloc`
and related functions is not physically contiguous.
diff --git a/include/linux/slab.h b/include/linux/slab.h
index dd6897f62010..be4ba5867ac5 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -592,6 +592,24 @@ static inline void *kmalloc_array(size_t n, size_t size,
gfp_t flags)
return __kmalloc(bytes, flags);
}
+/**
+ * krealloc_array - reallocate memory for an array.
+ * @p: pointer to the memory chunk to reallocate
+ * @new_n: new number of elements to alloc
+ * @new_size: new size of a single member of the array
+ * @flags: the type of memory to allocate (see kmalloc)
+ */
+static __must_check inline void *
+krealloc_array(void *p, size_t new_n, size_t new_size, gfp_t flags)
+{
+ size_t bytes;
+
+ if (unlikely(check_mul_overflow(new_n, new_size, )))
+ return NULL;
+
+ return krealloc(p, bytes, flags);
+}
+
/**
* kcalloc - allocate memory for an array. The memory is set to zero.
* @n: number of elements.
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/9] ALSA: pcm: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Takashi Iwai
---
sound/core/pcm_lib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index bda3514c7b2d..b7e3d8f44511 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1129,8 +1129,8 @@ int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime,
unsigned int cond,
if (constrs->rules_num >= constrs->rules_all) {
struct snd_pcm_hw_rule *new;
unsigned int new_rules = constrs->rules_all + 16;
- new = krealloc(constrs->rules, new_rules * sizeof(*c),
- GFP_KERNEL);
+ new = krealloc_array(constrs->rules, new_rules,
+sizeof(*c), GFP_KERNEL);
if (!new) {
va_end(args);
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/9] mm: slab: clarify krealloc()'s behavior with __GFP_ZERO
From: Bartosz Golaszewski __GFP_ZERO is ignored by krealloc() (unless we fall-back to kmalloc() path, in which case it's honored). Point that out in the kerneldoc. Signed-off-by: Bartosz Golaszewski --- mm/slab_common.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mm/slab_common.c b/mm/slab_common.c index f9ccd5dc13f3..d6df73f79204 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -1091,9 +1091,9 @@ static __always_inline void *__do_krealloc(const void *p, size_t new_size, * @flags: the type of memory to allocate. * * The contents of the object pointed to are preserved up to the - * lesser of the new and old sizes. If @p is %NULL, krealloc() - * behaves exactly like kmalloc(). If @new_size is 0 and @p is not a - * %NULL pointer, the object pointed to is freed. + * lesser of the new and old sizes (__GFP_ZERO flag is effectively ignored). + * If @p is %NULL, krealloc() behaves exactly like kmalloc(). If @new_size + * is 0 and @p is not a %NULL pointer, the object pointed to is freed. * * Return: pointer to the allocated memory or %NULL in case of error */ -- 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/9] slab: provide and use krealloc_array()
From: Bartosz Golaszewski Andy brought to my attention the fact that users allocating an array of equally sized elements should check if the size multiplication doesn't overflow. This is why we have helpers like kmalloc_array(). However we don't have krealloc_array() equivalent and there are many users who do their own multiplication when calling krealloc() for arrays. This series provides krealloc_array() and uses it in a couple places. A separate series will follow adding devm_krealloc_array() which is needed in the xilinx adc driver. v1 -> v2: - added a kernel doc for krealloc_array() - mentioned krealloc et al in the docs - collected review tags v2 -> v3: - add a patch improving krealloc()'s kerneldoc - fix a typo - improve .rst doc - tweak line breaks Bartosz Golaszewski (9): mm: slab: clarify krealloc()'s behavior with __GFP_ZERO mm: slab: provide krealloc_array() ALSA: pcm: use krealloc_array() vhost: vringh: use krealloc_array() pinctrl: use krealloc_array() edac: ghes: use krealloc_array() drm: atomic: use krealloc_array() hwtracing: intel: use krealloc_array() dma-buf: use krealloc_array() Documentation/core-api/memory-allocation.rst | 4 drivers/dma-buf/sync_file.c | 3 +-- drivers/edac/ghes_edac.c | 4 ++-- drivers/gpu/drm/drm_atomic.c | 3 ++- drivers/hwtracing/intel_th/msu.c | 2 +- drivers/pinctrl/pinctrl-utils.c | 2 +- drivers/vhost/vringh.c | 3 ++- include/linux/slab.h | 18 ++ mm/slab_common.c | 6 +++--- sound/core/pcm_lib.c | 4 ++-- 10 files changed, 36 insertions(+), 13 deletions(-) -- 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/8] slab: provide and use krealloc_array()
On Tue, Nov 3, 2020 at 5:14 AM Joe Perches wrote: > > On Mon, 2020-11-02 at 16:20 +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > > > Andy brought to my attention the fact that users allocating an array of > > equally sized elements should check if the size multiplication doesn't > > overflow. This is why we have helpers like kmalloc_array(). > > > > However we don't have krealloc_array() equivalent and there are many > > users who do their own multiplication when calling krealloc() for arrays. > > > > This series provides krealloc_array() and uses it in a couple places. > > My concern about this is a possible assumption that __GFP_ZERO will > work, and as far as I know, it will not. > Yeah so I had this concern for devm_krealloc() and even sent a patch that extended it to honor __GFP_ZERO before I noticed that regular krealloc() silently ignores __GFP_ZERO. I'm not sure if this is on purpose. Maybe we should either make krealloc() honor __GFP_ZERO or explicitly state in its documentation that it ignores it? This concern isn't really related to this patch as such - it's more of a general krealloc() inconsistency. Bartosz ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/8] mm: slab: provide krealloc_array()
On Mon, Nov 2, 2020 at 4:41 PM Matthew Wilcox wrote: > > On Mon, Nov 02, 2020 at 04:20:30PM +0100, Bartosz Golaszewski wrote: > > +Chunks allocated with `kmalloc` can be resized with `krealloc`. Similarly > > +to `kmalloc_array`: a helper for resising arrays is provided in the form of > > +`krealloc_array`. > > Is there any reason you chose to `do_this` instead of do_this()? The > automarkup script turns do_this() into a nice link to the documentation > which you're adding below. > No, I just didn't know better. Thanks for bringing this to my attention. > Typo 'resising' resizing. Will fix in the next iteration. Bartosz ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 6/8] drm: atomic: use krealloc_array()
From: Bartosz Golaszewski Use the helper that checks for overflows internally instead of manually calculating the size of the new array. Signed-off-by: Bartosz Golaszewski Acked-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 58527f151984..09ad6a2ec17b 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -960,7 +960,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, struct __drm_connnectors_state *c; int alloc = max(index + 1, config->num_connector); - c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL); + c = krealloc_array(state->connectors, alloc, + sizeof(*state->connectors), GFP_KERNEL); if (!c) return ERR_PTR(-ENOMEM); -- 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 8/8] dma-buf: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Acked-by: Christian König
---
drivers/dma-buf/sync_file.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 5a5a1da01a00..2925ea03eef0 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -270,8 +270,8 @@ static struct sync_file *sync_file_merge(const char *name,
struct sync_file *a,
fences[i++] = dma_fence_get(a_fences[0]);
if (num_fences > i) {
- nfences = krealloc(fences, i * sizeof(*fences),
- GFP_KERNEL);
+ nfences = krealloc_array(fences, i,
+sizeof(*fences), GFP_KERNEL);
if (!nfences)
goto err;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/8] ALSA: pcm: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Takashi Iwai
---
sound/core/pcm_lib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index d531e1bc2b81..c6f8f9e252e0 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1129,8 +1129,8 @@ int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime,
unsigned int cond,
if (constrs->rules_num >= constrs->rules_all) {
struct snd_pcm_hw_rule *new;
unsigned int new_rules = constrs->rules_all + 16;
- new = krealloc(constrs->rules, new_rules * sizeof(*c),
- GFP_KERNEL);
+ new = krealloc_array(constrs->rules, new_rules,
+sizeof(*c), GFP_KERNEL);
if (!new) {
va_end(args);
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/8] pinctrl: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
---
drivers/pinctrl/pinctrl-utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-utils.c b/drivers/pinctrl/pinctrl-utils.c
index f2bcbf62c03d..93df0d4c0a24 100644
--- a/drivers/pinctrl/pinctrl-utils.c
+++ b/drivers/pinctrl/pinctrl-utils.c
@@ -39,7 +39,7 @@ int pinctrl_utils_reserve_map(struct pinctrl_dev *pctldev,
if (old_num >= new_num)
return 0;
- new_map = krealloc(*map, sizeof(*new_map) * new_num, GFP_KERNEL);
+ new_map = krealloc_array(*map, new_num, sizeof(*new_map), GFP_KERNEL);
if (!new_map) {
dev_err(pctldev->dev, "krealloc(map) failed\n");
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/8] slab: provide and use krealloc_array()
From: Bartosz Golaszewski Andy brought to my attention the fact that users allocating an array of equally sized elements should check if the size multiplication doesn't overflow. This is why we have helpers like kmalloc_array(). However we don't have krealloc_array() equivalent and there are many users who do their own multiplication when calling krealloc() for arrays. This series provides krealloc_array() and uses it in a couple places. A separate series will follow adding devm_krealloc_array() which is needed in the xilinx adc driver. v1 -> v2: - added a kernel doc for krealloc_array() - mentioned krealloc et al in the docs - collected review tags Bartosz Golaszewski (8): mm: slab: provide krealloc_array() ALSA: pcm: use krealloc_array() vhost: vringh: use krealloc_array() pinctrl: use krealloc_array() edac: ghes: use krealloc_array() drm: atomic: use krealloc_array() hwtracing: intel: use krealloc_array() dma-buf: use krealloc_array() Documentation/core-api/memory-allocation.rst | 4 drivers/dma-buf/sync_file.c | 4 ++-- drivers/edac/ghes_edac.c | 4 ++-- drivers/gpu/drm/drm_atomic.c | 3 ++- drivers/hwtracing/intel_th/msu.c | 2 +- drivers/pinctrl/pinctrl-utils.c | 2 +- drivers/vhost/vringh.c | 3 ++- include/linux/slab.h | 18 ++ sound/core/pcm_lib.c | 4 ++-- 9 files changed, 34 insertions(+), 10 deletions(-) -- 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 7/8] hwtracing: intel: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
drivers/hwtracing/intel_th/msu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 3a77551fb4fc..7d95242db900 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -2002,7 +2002,7 @@ nr_pages_store(struct device *dev, struct
device_attribute *attr,
}
nr_wins++;
- rewin = krealloc(win, sizeof(*win) * nr_wins, GFP_KERNEL);
+ rewin = krealloc_array(win, nr_wins, sizeof(*win), GFP_KERNEL);
if (!rewin) {
kfree(win);
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/8] edac: ghes: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Acked-by: Borislav Petkov
---
drivers/edac/ghes_edac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index a918ca93e4f7..6d1ddecbf0da 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -207,8 +207,8 @@ static void enumerate_dimms(const struct dmi_header *dh,
void *arg)
if (!hw->num_dimms || !(hw->num_dimms % 16)) {
struct dimm_info *new;
- new = krealloc(hw->dimms, (hw->num_dimms + 16) * sizeof(struct
dimm_info),
- GFP_KERNEL);
+ new = krealloc_array(hw->dimms, hw->num_dimms + 16,
+sizeof(struct dimm_info), GFP_KERNEL);
if (!new) {
WARN_ON_ONCE(1);
return;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/8] mm: slab: provide krealloc_array()
From: Bartosz Golaszewski
When allocating an array of elements, users should check for
multiplication overflow or preferably use one of the provided helpers
like: kmalloc_array().
There's no krealloc_array() counterpart but there are many users who use
regular krealloc() to reallocate arrays. Let's provide an actual
krealloc_array() implementation.
While at it: add some documentation regarding krealloc.
Signed-off-by: Bartosz Golaszewski
Acked-by: Vlastimil Babka
---
Documentation/core-api/memory-allocation.rst | 4
include/linux/slab.h | 18 ++
2 files changed, 22 insertions(+)
diff --git a/Documentation/core-api/memory-allocation.rst
b/Documentation/core-api/memory-allocation.rst
index 4446a1ac36cc..6dc38b40439a 100644
--- a/Documentation/core-api/memory-allocation.rst
+++ b/Documentation/core-api/memory-allocation.rst
@@ -147,6 +147,10 @@ The address of a chunk allocated with `kmalloc` is aligned
to at least
ARCH_KMALLOC_MINALIGN bytes. For sizes which are a power of two, the
alignment is also guaranteed to be at least the respective size.
+Chunks allocated with `kmalloc` can be resized with `krealloc`. Similarly
+to `kmalloc_array`: a helper for resising arrays is provided in the form of
+`krealloc_array`.
+
For large allocations you can use vmalloc() and vzalloc(), or directly
request pages from the page allocator. The memory allocated by `vmalloc`
and related functions is not physically contiguous.
diff --git a/include/linux/slab.h b/include/linux/slab.h
index dd6897f62010..be4ba5867ac5 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -592,6 +592,24 @@ static inline void *kmalloc_array(size_t n, size_t size,
gfp_t flags)
return __kmalloc(bytes, flags);
}
+/**
+ * krealloc_array - reallocate memory for an array.
+ * @p: pointer to the memory chunk to reallocate
+ * @new_n: new number of elements to alloc
+ * @new_size: new size of a single member of the array
+ * @flags: the type of memory to allocate (see kmalloc)
+ */
+static __must_check inline void *
+krealloc_array(void *p, size_t new_n, size_t new_size, gfp_t flags)
+{
+ size_t bytes;
+
+ if (unlikely(check_mul_overflow(new_n, new_size, )))
+ return NULL;
+
+ return krealloc(p, bytes, flags);
+}
+
/**
* kcalloc - allocate memory for an array. The memory is set to zero.
* @n: number of elements.
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/8] vhost: vringh: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
Acked-by: Michael S. Tsirkin
---
drivers/vhost/vringh.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 8bd8b403f087..08a0e1c842df 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -198,7 +198,8 @@ static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
if (flag)
- new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
+ new = krealloc_array(iov->iov, new_num,
+sizeof(struct iovec), gfp);
else {
new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
if (new) {
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] vhost: vringh: use krealloc_array()
On Tue, Oct 27, 2020 at 6:08 PM Joe Perches wrote:
>
> On Tue, 2020-10-27 at 17:58 +0100, Bartosz Golaszewski wrote:
> > On Tue, Oct 27, 2020 at 5:50 PM Joe Perches wrote:
> > >
> > > On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote:
> > > > On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
> > > > > From: Bartosz Golaszewski
> > > > >
> > > > > Use the helper that checks for overflows internally instead of
> > > > > manually
> > > > > calculating the size of the new array.
> > > > >
> > > > > Signed-off-by: Bartosz Golaszewski
> > > >
> > > > No problem with the patch, it does introduce some symmetry in the code.
> > >
> > > Perhaps more symmetry by using kmemdup
> > > ---
> > > drivers/vhost/vringh.c | 23 ++-
> > > 1 file changed, 10 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > > index 8bd8b403f087..99222a3651cd 100644
> > > --- a/drivers/vhost/vringh.c
> > > +++ b/drivers/vhost/vringh.c
> > > @@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh
> > > *vrh,
> > > static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
> > > {
> > > struct kvec *new;
> > > - unsigned int flag, new_num = (iov->max_num &
> > > ~VRINGH_IOV_ALLOCATED) * 2;
> > > + size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> > > + size_t size;
> > >
> > > if (new_num < 8)
> > > new_num = 8;
> > >
> > > - flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
> > > - if (flag)
> > > - new = krealloc(iov->iov, new_num * sizeof(struct iovec),
> > > gfp);
> > > - else {
> > > - new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
> > > - if (new) {
> > > - memcpy(new, iov->iov,
> > > - iov->max_num * sizeof(struct iovec));
> > > - flag = VRINGH_IOV_ALLOCATED;
> > > - }
> > > - }
> > > + if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec),
> > > )))
> > > + return -ENOMEM;
> > > +
> >
> > The whole point of using helpers such as kmalloc_array() is not doing
> > these checks manually.
>
> Tradeoffs for in readability for overflow and not mistyping or doing
> the multiplication of iov->max_num * sizeof(struct iovec) twice.
>
It's out of scope for this series - I want to add users for
krealloc_array(), not refactor code I don't really know. If the
maintainer of this bit objects, it can be dropped.
> Just fyi:
>
> the realloc doesn't do a multiplication overflow test as written so the
> suggestion is slightly more resistant to defect.
>
I'm not sure what your point is. I used krealloc_array() exactly for
this reason - to add the overflow test.
BTW I suppose kmalloc_array() here can be replaced with
krealloc_array() if the original pointer is NULL the first time it's
called.
Bartosz
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/8] mm: slab: provide krealloc_array()
From: Bartosz Golaszewski
When allocating an array of elements, users should check for
multiplication overflow or preferably use one of the provided helpers
like: kmalloc_array().
There's no krealloc_array() counterpart but there are many users who use
regular krealloc() to reallocate arrays. Let's provide an actual
krealloc_array() implementation.
Signed-off-by: Bartosz Golaszewski
---
include/linux/slab.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index dd6897f62010..0e6683affee7 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -592,6 +592,17 @@ static inline void *kmalloc_array(size_t n, size_t size,
gfp_t flags)
return __kmalloc(bytes, flags);
}
+static __must_check inline void *
+krealloc_array(void *p, size_t new_n, size_t new_size, gfp_t flags)
+{
+ size_t bytes;
+
+ if (unlikely(check_mul_overflow(new_n, new_size, )))
+ return NULL;
+
+ return krealloc(p, bytes, flags);
+}
+
/**
* kcalloc - allocate memory for an array. The memory is set to zero.
* @n: number of elements.
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/8] ALSA: pcm: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
sound/core/pcm_lib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index d531e1bc2b81..c6f8f9e252e0 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1129,8 +1129,8 @@ int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime,
unsigned int cond,
if (constrs->rules_num >= constrs->rules_all) {
struct snd_pcm_hw_rule *new;
unsigned int new_rules = constrs->rules_all + 16;
- new = krealloc(constrs->rules, new_rules * sizeof(*c),
- GFP_KERNEL);
+ new = krealloc_array(constrs->rules, new_rules,
+sizeof(*c), GFP_KERNEL);
if (!new) {
va_end(args);
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/8] slab: provide and use krealloc_array()
From: Bartosz Golaszewski Andy brought to my attention the fact that users allocating an array of equally sized elements should check if the size multiplication doesn't overflow. This is why we have helpers like kmalloc_array(). However we don't have krealloc_array() equivalent and there are many users who do their own multiplication when calling krealloc() for arrays. This series provides krealloc_array() and uses it in a couple places. A separate series will follow adding devm_krealloc_array() which is needed in the xilinx adc driver. Bartosz Golaszewski (8): mm: slab: provide krealloc_array() ALSA: pcm: use krealloc_array() vhost: vringh: use krealloc_array() pinctrl: use krealloc_array() edac: ghes: use krealloc_array() drm: atomic: use krealloc_array() hwtracing: intel: use krealloc_array() dma-buf: use krealloc_array() drivers/dma-buf/sync_file.c | 4 ++-- drivers/edac/ghes_edac.c | 4 ++-- drivers/gpu/drm/drm_atomic.c | 3 ++- drivers/hwtracing/intel_th/msu.c | 2 +- drivers/pinctrl/pinctrl-utils.c | 2 +- drivers/vhost/vringh.c | 3 ++- include/linux/slab.h | 11 +++ sound/core/pcm_lib.c | 4 ++-- 8 files changed, 23 insertions(+), 10 deletions(-) -- 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 8/8] dma-buf: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
drivers/dma-buf/sync_file.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 5a5a1da01a00..2925ea03eef0 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -270,8 +270,8 @@ static struct sync_file *sync_file_merge(const char *name,
struct sync_file *a,
fences[i++] = dma_fence_get(a_fences[0]);
if (num_fences > i) {
- nfences = krealloc(fences, i * sizeof(*fences),
- GFP_KERNEL);
+ nfences = krealloc_array(fences, i,
+sizeof(*fences), GFP_KERNEL);
if (!nfences)
goto err;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/8] edac: ghes: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
drivers/edac/ghes_edac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index a918ca93e4f7..6d1ddecbf0da 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -207,8 +207,8 @@ static void enumerate_dimms(const struct dmi_header *dh,
void *arg)
if (!hw->num_dimms || !(hw->num_dimms % 16)) {
struct dimm_info *new;
- new = krealloc(hw->dimms, (hw->num_dimms + 16) * sizeof(struct
dimm_info),
- GFP_KERNEL);
+ new = krealloc_array(hw->dimms, hw->num_dimms + 16,
+sizeof(struct dimm_info), GFP_KERNEL);
if (!new) {
WARN_ON_ONCE(1);
return;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/8] hwtracing: intel: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
drivers/hwtracing/intel_th/msu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 3a77551fb4fc..7d95242db900 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -2002,7 +2002,7 @@ nr_pages_store(struct device *dev, struct
device_attribute *attr,
}
nr_wins++;
- rewin = krealloc(win, sizeof(*win) * nr_wins, GFP_KERNEL);
+ rewin = krealloc_array(win, nr_wins, sizeof(*win), GFP_KERNEL);
if (!rewin) {
kfree(win);
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/8] pinctrl: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
drivers/pinctrl/pinctrl-utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-utils.c b/drivers/pinctrl/pinctrl-utils.c
index f2bcbf62c03d..93df0d4c0a24 100644
--- a/drivers/pinctrl/pinctrl-utils.c
+++ b/drivers/pinctrl/pinctrl-utils.c
@@ -39,7 +39,7 @@ int pinctrl_utils_reserve_map(struct pinctrl_dev *pctldev,
if (old_num >= new_num)
return 0;
- new_map = krealloc(*map, sizeof(*new_map) * new_num, GFP_KERNEL);
+ new_map = krealloc_array(*map, new_num, sizeof(*new_map), GFP_KERNEL);
if (!new_map) {
dev_err(pctldev->dev, "krealloc(map) failed\n");
return -ENOMEM;
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] vhost: vringh: use krealloc_array()
On Tue, Oct 27, 2020 at 5:50 PM Joe Perches wrote:
>
> On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote:
> > On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski
> > >
> > > Use the helper that checks for overflows internally instead of manually
> > > calculating the size of the new array.
> > >
> > > Signed-off-by: Bartosz Golaszewski
> >
> > No problem with the patch, it does introduce some symmetry in the code.
>
> Perhaps more symmetry by using kmemdup
> ---
> drivers/vhost/vringh.c | 23 ++-
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 8bd8b403f087..99222a3651cd 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh,
> static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
> {
> struct kvec *new;
> - unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) *
> 2;
> + size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> + size_t size;
>
> if (new_num < 8)
> new_num = 8;
>
> - flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
> - if (flag)
> - new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
> - else {
> - new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
> - if (new) {
> - memcpy(new, iov->iov,
> - iov->max_num * sizeof(struct iovec));
> - flag = VRINGH_IOV_ALLOCATED;
> - }
> - }
> + if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec),
> )))
> + return -ENOMEM;
> +
The whole point of using helpers such as kmalloc_array() is not doing
these checks manually.
Bartosz
> + if (iov->max_num & VRINGH_IOV_ALLOCATED)
> + new = krealloc(iov->iov, size, gfp);
> + else
> + new = kmemdup(iov->iov, size, gfp);
> if (!new)
> return -ENOMEM;
> iov->iov = new;
> - iov->max_num = (new_num | flag);
> + iov->max_num = new_num | VRINGH_IOV_ALLOCATED;
> return 0;
> }
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/8] drm: atomic: use krealloc_array()
From: Bartosz Golaszewski Use the helper that checks for overflows internally instead of manually calculating the size of the new array. Signed-off-by: Bartosz Golaszewski --- drivers/gpu/drm/drm_atomic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 58527f151984..09ad6a2ec17b 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -960,7 +960,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, struct __drm_connnectors_state *c; int alloc = max(index + 1, config->num_connector); - c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL); + c = krealloc_array(state->connectors, alloc, + sizeof(*state->connectors), GFP_KERNEL); if (!c) return ERR_PTR(-ENOMEM); -- 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/8] vhost: vringh: use krealloc_array()
From: Bartosz Golaszewski
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski
---
drivers/vhost/vringh.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 8bd8b403f087..08a0e1c842df 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -198,7 +198,8 @@ static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
if (flag)
- new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
+ new = krealloc_array(iov->iov, new_num,
+sizeof(struct iovec), gfp);
else {
new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
if (new) {
--
2.29.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: pwm_bl: Switch to full GPIO descriptor
czw., 5 gru 2019 o 09:11 Linus Walleij napisał(a):
>
> The PWM backlight still supports passing a enable GPIO line as
> platform data using the legacy API.
>
> It turns out that ever board using this mechanism except one
> is pass .enable_gpio = -1. So we drop all these cargo-culted -1's
> from all instances of this platform data in the kernel.
>
> The remaning board, Palm TC, is converted to pass a machine
> descriptior table with the "enable" GPIO instead, and delete the
> platform data entry for enable_gpio and the code handling it
> and things should work smoothly with the new API.
>
> Cc: Krzysztof Kozlowski
> Cc: Robert Jarzmik
> Cc: Guan Xuetao
> Cc: Bartosz Golaszewski
> Signed-off-by: Linus Walleij
> ---
> arch/arm/mach-pxa/cm-x300.c | 1 -
> arch/arm/mach-pxa/colibri-pxa270-income.c | 1 -
> arch/arm/mach-pxa/ezx.c | 1 -
> arch/arm/mach-pxa/hx4700.c| 1 -
> arch/arm/mach-pxa/lpd270.c| 1 -
> arch/arm/mach-pxa/magician.c | 1 -
> arch/arm/mach-pxa/mainstone.c | 1 -
> arch/arm/mach-pxa/mioa701.c | 1 -
> arch/arm/mach-pxa/palm27x.c | 1 -
> arch/arm/mach-pxa/palmtc.c| 10 ++
> arch/arm/mach-pxa/palmte2.c | 1 -
> arch/arm/mach-pxa/pcm990-baseboard.c | 1 -
> arch/arm/mach-pxa/tavorevb.c | 2 --
> arch/arm/mach-pxa/viper.c | 1 -
> arch/arm/mach-pxa/z2.c| 2 --
> arch/arm/mach-pxa/zylonite.c | 1 -
> arch/arm/mach-s3c24xx/mach-h1940.c| 1 -
> arch/arm/mach-s3c24xx/mach-rx1950.c | 1 -
> arch/arm/mach-s3c64xx/dev-backlight.c | 3 ---
> arch/arm/mach-s3c64xx/mach-crag6410.c | 1 -
> arch/arm/mach-s3c64xx/mach-hmt.c | 1 -
> arch/arm/mach-s3c64xx/mach-smartq.c | 1 -
> arch/arm/mach-s3c64xx/mach-smdk6410.c | 2 +-
> arch/unicore32/kernel/puv3-nb0916.c | 1 -
> drivers/video/backlight/pwm_bl.c | 19 ---
> include/linux/pwm_backlight.h | 2 --
> 26 files changed, 11 insertions(+), 48 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
> index 425855f456f2..2e35354b61f5 100644
> --- a/arch/arm/mach-pxa/cm-x300.c
> +++ b/arch/arm/mach-pxa/cm-x300.c
> @@ -312,7 +312,6 @@ static struct pwm_lookup cm_x300_pwm_lookup[] = {
> static struct platform_pwm_backlight_data cm_x300_backlight_data = {
> .max_brightness = 100,
> .dft_brightness = 100,
> - .enable_gpio= -1,
> };
>
> static struct platform_device cm_x300_backlight_device = {
> diff --git a/arch/arm/mach-pxa/colibri-pxa270-income.c
> b/arch/arm/mach-pxa/colibri-pxa270-income.c
> index dbad2f13706c..e5879e8b0682 100644
> --- a/arch/arm/mach-pxa/colibri-pxa270-income.c
> +++ b/arch/arm/mach-pxa/colibri-pxa270-income.c
> @@ -202,7 +202,6 @@ static struct pwm_lookup income_pwm_lookup[] = {
> static struct platform_pwm_backlight_data income_backlight_data = {
> .max_brightness = 0x3ff,
> .dft_brightness = 0x1ff,
> - .enable_gpio= -1,
> };
>
> static struct platform_device income_backlight = {
> diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
> index ec10851b63cf..eb85950e7c0e 100644
> --- a/arch/arm/mach-pxa/ezx.c
> +++ b/arch/arm/mach-pxa/ezx.c
> @@ -55,7 +55,6 @@ static struct pwm_lookup ezx_pwm_lookup[] __maybe_unused = {
> static struct platform_pwm_backlight_data ezx_backlight_data = {
> .max_brightness = 1023,
> .dft_brightness = 1023,
> - .enable_gpio= -1,
> };
>
> static struct platform_device ezx_backlight_device = {
> diff --git a/arch/arm/mach-pxa/hx4700.c b/arch/arm/mach-pxa/hx4700.c
> index 311268d186ab..60186224df49 100644
> --- a/arch/arm/mach-pxa/hx4700.c
> +++ b/arch/arm/mach-pxa/hx4700.c
> @@ -557,7 +557,6 @@ static struct platform_device hx4700_lcd = {
> static struct platform_pwm_backlight_data backlight_data = {
> .max_brightness = 200,
> .dft_brightness = 100,
> - .enable_gpio= -1,
> };
>
> static struct platform_device backlight = {
> diff --git a/arch/arm/mach-pxa/lpd270.c b/arch/arm/mach-pxa/lpd270.c
> index 20e00e970385..6fc40bc06910 100644
> --- a/arch/arm/mach-pxa/lpd270.c
> +++ b/arch/arm/mach-pxa/lpd270.c
> @@ -277,7 +277,6 @@ static struct pwm_lookup lpd270_pwm_lookup[] = {
> static struct platform_pwm_backlight_data lpd270_backlight_data = {
> .max_brightness = 1,
> .dft_brightness = 1,
> - .enable_gpio= -1,
> };
>
> static struct platform_device lpd270_backlight_devic
Re: [PATCH v7 0/9] backlight: gpio: simplify the driver
pon., 4 lis 2019 o 10:22 Bartosz Golaszewski napisał(a):
>
> pt., 1 lis 2019 o 16:39 Jacopo Mondi napisał(a):
> >
> > Hello,
> > as promised...
> >
> > On Fri, Nov 01, 2019 at 08:58:03AM +, Lee Jones wrote:
> > > On Thu, 24 Oct 2019, Jacopo Mondi wrote:
> > >
> > > > Hello,
> > > >
> > > > On Thu, Oct 24, 2019 at 07:47:26AM +0100, Lee Jones wrote:
> > > > > On Wed, 23 Oct 2019, Daniel Thompson wrote:
> > > > >
> > > > > > On Tue, Oct 22, 2019 at 11:29:54AM +0200, Bartosz Golaszewski wrote:
> > > > > > > wt., 22 paź 2019 o 10:36 Bartosz Golaszewski
> > > > > > > napisał(a):
> > > > > > > >
> > > > > > > > From: Bartosz Golaszewski
> > > > > > > >
> > > > > > > > While working on my other series related to gpio-backlight[1] I
> > > > > > > > noticed
> > > > > > > > that we could simplify the driver if we made the only user of
> > > > > > > > platform
> > > > > > > > data use GPIO lookups and device properties. This series tries
> > > > > > > > to do
> > > > > > > > that.
> > > > > > > >
> > > > > > > > First two patches contain minor fixes. Third patch makes the
> > > > > > > > driver
> > > > > > > > explicitly drive the GPIO line. Fourth patch adds all necessary
> > > > > > > > data
> > > > > > > > structures to ecovec24. Patch 5/9 unifies much of the code for
> > > > > > > > both
> > > > > > > > pdata and non-pdata cases. Patches 6-7/9 remove unused platform
> > > > > > > > data
> > > > > > > > fields. Last two patches contain additional improvements for
> > > > > > > > the GPIO
> > > > > > > > backlight driver while we're already modifying it.
> > > > > > > >
> > > > > > > > I don't have access to this HW but hopefully this works. Only
> > > > > > > > compile
> > > > > > > > tested.
> > > > > > > >
> > > > > > > > [1] https://lkml.org/lkml/2019/6/25/900
> > > > > > > >
> > > > > > > > v1 -> v2:
> > > > > > > > - rebased on top of v5.3-rc1 and adjusted to the recent changes
> > > > > > > > from Andy
> > > > > > > > - added additional two patches with minor improvements
> > > > > > > >
> > > > > > > > v2 -> v3:
> > > > > > > > - in patch 7/7: used initializers to set values for pdata and
> > > > > > > > dev local vars
> > > > > > > >
> > > > > > > > v3 -> v4:
> > > > > > > > - rebased on top of v5.4-rc1
> > > > > > > > - removed changes that are no longer relevant after commit
> > > > > > > > ec665b756e6f
> > > > > > > > ("backlight: gpio-backlight: Correct initial power state
> > > > > > > > handling")
> > > > > > > > - added patch 7/7
> > > > > > > >
> > > > > > > > v4 -> v5:
> > > > > > > > - in patch 7/7: added a comment replacing the name of the
> > > > > > > > function being
> > > > > > > > pulled into probe()
> > > > > > > >
> > > > > > > > v5 -> v6:
> > > > > > > > - added a patch making the driver explicitly set the direction
> > > > > > > > of the GPIO
> > > > > > > > to output
> > > > > > > > - added a patch removing a redundant newline
> > > > > > > >
> > > > > > > > v6 -> v7:
> > > > > > > > - renamed the function calculating the new GPIO value for
> > > > > > > > status update
> > > > > > > > - collected more tags
> > > > > > > >
> > > > > > > > Bartosz Golaszewski (9):
> > > > > > > > backlight: gpio: remove unneeded include
> > >
Re: [PATCH v7 0/9] backlight: gpio: simplify the driver
pt., 1 lis 2019 o 16:39 Jacopo Mondi napisał(a):
>
> Hello,
> as promised...
>
> On Fri, Nov 01, 2019 at 08:58:03AM +, Lee Jones wrote:
> > On Thu, 24 Oct 2019, Jacopo Mondi wrote:
> >
> > > Hello,
> > >
> > > On Thu, Oct 24, 2019 at 07:47:26AM +0100, Lee Jones wrote:
> > > > On Wed, 23 Oct 2019, Daniel Thompson wrote:
> > > >
> > > > > On Tue, Oct 22, 2019 at 11:29:54AM +0200, Bartosz Golaszewski wrote:
> > > > > > wt., 22 paź 2019 o 10:36 Bartosz Golaszewski
> > > > > > napisał(a):
> > > > > > >
> > > > > > > From: Bartosz Golaszewski
> > > > > > >
> > > > > > > While working on my other series related to gpio-backlight[1] I
> > > > > > > noticed
> > > > > > > that we could simplify the driver if we made the only user of
> > > > > > > platform
> > > > > > > data use GPIO lookups and device properties. This series tries to
> > > > > > > do
> > > > > > > that.
> > > > > > >
> > > > > > > First two patches contain minor fixes. Third patch makes the
> > > > > > > driver
> > > > > > > explicitly drive the GPIO line. Fourth patch adds all necessary
> > > > > > > data
> > > > > > > structures to ecovec24. Patch 5/9 unifies much of the code for
> > > > > > > both
> > > > > > > pdata and non-pdata cases. Patches 6-7/9 remove unused platform
> > > > > > > data
> > > > > > > fields. Last two patches contain additional improvements for the
> > > > > > > GPIO
> > > > > > > backlight driver while we're already modifying it.
> > > > > > >
> > > > > > > I don't have access to this HW but hopefully this works. Only
> > > > > > > compile
> > > > > > > tested.
> > > > > > >
> > > > > > > [1] https://lkml.org/lkml/2019/6/25/900
> > > > > > >
> > > > > > > v1 -> v2:
> > > > > > > - rebased on top of v5.3-rc1 and adjusted to the recent changes
> > > > > > > from Andy
> > > > > > > - added additional two patches with minor improvements
> > > > > > >
> > > > > > > v2 -> v3:
> > > > > > > - in patch 7/7: used initializers to set values for pdata and dev
> > > > > > > local vars
> > > > > > >
> > > > > > > v3 -> v4:
> > > > > > > - rebased on top of v5.4-rc1
> > > > > > > - removed changes that are no longer relevant after commit
> > > > > > > ec665b756e6f
> > > > > > > ("backlight: gpio-backlight: Correct initial power state
> > > > > > > handling")
> > > > > > > - added patch 7/7
> > > > > > >
> > > > > > > v4 -> v5:
> > > > > > > - in patch 7/7: added a comment replacing the name of the
> > > > > > > function being
> > > > > > > pulled into probe()
> > > > > > >
> > > > > > > v5 -> v6:
> > > > > > > - added a patch making the driver explicitly set the direction of
> > > > > > > the GPIO
> > > > > > > to output
> > > > > > > - added a patch removing a redundant newline
> > > > > > >
> > > > > > > v6 -> v7:
> > > > > > > - renamed the function calculating the new GPIO value for status
> > > > > > > update
> > > > > > > - collected more tags
> > > > > > >
> > > > > > > Bartosz Golaszewski (9):
> > > > > > > backlight: gpio: remove unneeded include
> > > > > > > backlight: gpio: remove stray newline
> > > > > > > backlight: gpio: explicitly set the direction of the GPIO
> > > > > > > sh: ecovec24: add additional properties to the backlight device
> > > > > > > backlight: gpio: simplify the platform data handling
> > > > > > > sh: ecovec24: don't set unused fields in platform data
> > > > > >
[PATCH v7 9/9] backlight: gpio: pull gpio_backlight_initial_power_state() into probe
From: Bartosz Golaszewski
The probe function in the gpio-backlight driver is quite short. If we
pull gpio_backlight_initial_power_state() into probe we can drop two
more fields from struct gpio_backlight and shrink the driver code.
Signed-off-by: Bartosz Golaszewski
Acked-by: Daniel Thompson
---
drivers/video/backlight/gpio_backlight.c | 38 +---
1 file changed, 14 insertions(+), 24 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index d6969fae25cd..75409ddfba3e 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -17,11 +17,8 @@
#include
struct gpio_backlight {
- struct device *dev;
struct device *fbdev;
-
struct gpio_desc *gpiod;
- int def_value;
};
static int gpio_backlight_get_next_brightness(struct backlight_device *bl)
@@ -60,41 +57,24 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
-{
- struct device_node *node = gbl->dev->of_node;
-
- /* Not booted with device tree or no phandle link to the node */
- if (!node || !node->phandle)
- return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
-
- /* if the enable GPIO is disabled, do not enable the backlight */
- if (gpiod_get_direction(gbl->gpiod) == 0 &&
- gpiod_get_value_cansleep(gbl->gpiod) == 0)
- return FB_BLANK_POWERDOWN;
-
- return FB_BLANK_UNBLANK;
-}
-
static int gpio_backlight_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
+ struct device_node *of_node = dev->of_node;
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
- int ret, init_brightness;
+ int ret, init_brightness, def_value;
gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = dev;
-
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(dev, "default-on");
+ def_value = device_property_read_bool(dev, "default-on");
gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
@@ -115,7 +95,17 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
return PTR_ERR(bl);
}
- bl->props.power = gpio_backlight_initial_power_state(gbl);
+ /* Set the initial power state */
+ if (!of_node || !of_node->phandle)
+ /* Not booted with device tree or no phandle link to the node */
+ bl->props.power = def_value ? FB_BLANK_UNBLANK
+ : FB_BLANK_POWERDOWN;
+ else if (gpiod_get_direction(gbl->gpiod) == 0 &&
+gpiod_get_value_cansleep(gbl->gpiod) == 0)
+ bl->props.power = FB_BLANK_POWERDOWN;
+ else
+ bl->props.power = FB_BLANK_UNBLANK;
+
bl->props.brightness = 1;
init_brightness = gpio_backlight_get_next_brightness(bl);
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 8/9] backlight: gpio: use a helper variable for >dev
From: Bartosz Golaszewski
Instead of dereferencing pdev each time, use a helper variable for
the associated device pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index ddc7d3b288b7..d6969fae25cd 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -78,29 +78,29 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
static int gpio_backlight_probe(struct platform_device *pdev)
{
- struct gpio_backlight_platform_data *pdata =
- dev_get_platdata(>dev);
+ struct device *dev = >dev;
+ struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
int ret, init_brightness;
- gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
+ gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = >dev;
+ gbl->dev = dev;
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(>dev, "default-on");
+ gbl->def_value = device_property_read_bool(dev, "default-on");
- gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
ret = PTR_ERR(gbl->gpiod);
if (ret != -EPROBE_DEFER)
- dev_err(>dev,
+ dev_err(dev,
"Error: The gpios parameter is missing or
invalid.\n");
return ret;
}
@@ -108,11 +108,10 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
memset(, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = 1;
- bl = devm_backlight_device_register(>dev, dev_name(>dev),
- >dev, gbl, _backlight_ops,
- );
+ bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
+ _backlight_ops, );
if (IS_ERR(bl)) {
- dev_err(>dev, "failed to register backlight\n");
+ dev_err(dev, "failed to register backlight\n");
return PTR_ERR(bl);
}
@@ -122,7 +121,7 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
init_brightness = gpio_backlight_get_next_brightness(bl);
ret = gpiod_direction_output(gbl->gpiod, init_brightness);
if (ret) {
- dev_err(>dev, "failed to set initial brightness\n");
+ dev_err(dev, "failed to set initial brightness\n");
return ret;
}
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 7/9] backlight: gpio: remove unused fields from platform data
From: Bartosz Golaszewski
Remove the platform data fields that nobody uses.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
---
include/linux/platform_data/gpio_backlight.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/include/linux/platform_data/gpio_backlight.h
b/include/linux/platform_data/gpio_backlight.h
index 34179d600360..1a8b5b1946fe 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h
@@ -9,9 +9,6 @@ struct device;
struct gpio_backlight_platform_data {
struct device *fbdev;
- int gpio;
- int def_value;
- const char *name;
};
#endif
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 2/9] backlight: gpio: remove stray newline
From: Bartosz Golaszewski
Remove a double newline from the driver.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Daniel Thompson
---
drivers/video/backlight/gpio_backlight.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 7e1990199fae..3955b513f2f8 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -91,7 +91,6 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
return FB_BLANK_UNBLANK;
}
-
static int gpio_backlight_probe(struct platform_device *pdev)
{
struct gpio_backlight_platform_data *pdata =
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 3/9] backlight: gpio: explicitly set the direction of the GPIO
From: Bartosz Golaszewski
The GPIO backlight driver currently requests the line 'as is', without
acively setting its direction. This can lead to problems: if the line
is in input mode by default, we won't be able to drive it later when
updating the status and also reading its initial value doesn't make
sense for backlight setting.
Request the line 'as is' initially, so that we can read its value
without affecting it but then change the direction to output explicitly
when setting the initial brightness.
Also: check the current direction and only read the value if it's output.
Signed-off-by: Bartosz Golaszewski
---
drivers/video/backlight/gpio_backlight.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 3955b513f2f8..52f17c9ca1c3 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -25,9 +25,8 @@ struct gpio_backlight {
int def_value;
};
-static int gpio_backlight_update_status(struct backlight_device *bl)
+static int gpio_backlight_get_next_brightness(struct backlight_device *bl)
{
- struct gpio_backlight *gbl = bl_get_data(bl);
int brightness = bl->props.brightness;
if (bl->props.power != FB_BLANK_UNBLANK ||
@@ -35,6 +34,14 @@ static int gpio_backlight_update_status(struct
backlight_device *bl)
bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
brightness = 0;
+ return brightness;
+}
+
+static int gpio_backlight_update_status(struct backlight_device *bl)
+{
+ struct gpio_backlight *gbl = bl_get_data(bl);
+ int brightness = gpio_backlight_get_next_brightness(bl);
+
gpiod_set_value_cansleep(gbl->gpiod, brightness);
return 0;
@@ -85,7 +92,8 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
/* if the enable GPIO is disabled, do not enable the backlight */
- if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
+ if (gpiod_get_direction(gbl->gpiod) == 0 &&
+ gpiod_get_value_cansleep(gbl->gpiod) == 0)
return FB_BLANK_POWERDOWN;
return FB_BLANK_UNBLANK;
@@ -98,7 +106,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
- int ret;
+ int ret, init_brightness;
gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
@@ -151,7 +159,12 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
bl->props.power = gpio_backlight_initial_power_state(gbl);
bl->props.brightness = 1;
- backlight_update_status(bl);
+ init_brightness = gpio_backlight_get_next_brightness(bl);
+ ret = gpiod_direction_output(gbl->gpiod, init_brightness);
+ if (ret) {
+ dev_err(>dev, "failed to set initial brightness\n");
+ return ret;
+ }
platform_set_drvdata(pdev, bl);
return 0;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 0/9] backlight: gpio: simplify the driver
wt., 22 paź 2019 o 10:36 Bartosz Golaszewski napisał(a):
>
> From: Bartosz Golaszewski
>
> While working on my other series related to gpio-backlight[1] I noticed
> that we could simplify the driver if we made the only user of platform
> data use GPIO lookups and device properties. This series tries to do
> that.
>
> First two patches contain minor fixes. Third patch makes the driver
> explicitly drive the GPIO line. Fourth patch adds all necessary data
> structures to ecovec24. Patch 5/9 unifies much of the code for both
> pdata and non-pdata cases. Patches 6-7/9 remove unused platform data
> fields. Last two patches contain additional improvements for the GPIO
> backlight driver while we're already modifying it.
>
> I don't have access to this HW but hopefully this works. Only compile
> tested.
>
> [1] https://lkml.org/lkml/2019/6/25/900
>
> v1 -> v2:
> - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
> - added additional two patches with minor improvements
>
> v2 -> v3:
> - in patch 7/7: used initializers to set values for pdata and dev local vars
>
> v3 -> v4:
> - rebased on top of v5.4-rc1
> - removed changes that are no longer relevant after commit ec665b756e6f
> ("backlight: gpio-backlight: Correct initial power state handling")
> - added patch 7/7
>
> v4 -> v5:
> - in patch 7/7: added a comment replacing the name of the function being
> pulled into probe()
>
> v5 -> v6:
> - added a patch making the driver explicitly set the direction of the GPIO
> to output
> - added a patch removing a redundant newline
>
> v6 -> v7:
> - renamed the function calculating the new GPIO value for status update
> - collected more tags
>
> Bartosz Golaszewski (9):
> backlight: gpio: remove unneeded include
> backlight: gpio: remove stray newline
> backlight: gpio: explicitly set the direction of the GPIO
> sh: ecovec24: add additional properties to the backlight device
> backlight: gpio: simplify the platform data handling
> sh: ecovec24: don't set unused fields in platform data
> backlight: gpio: remove unused fields from platform data
> backlight: gpio: use a helper variable for >dev
> backlight: gpio: pull gpio_backlight_initial_power_state() into probe
>
> arch/sh/boards/mach-ecovec24/setup.c | 33 +++--
> drivers/video/backlight/gpio_backlight.c | 128 +++
> include/linux/platform_data/gpio_backlight.h | 3 -
> 3 files changed, 69 insertions(+), 95 deletions(-)
>
> --
> 2.23.0
>
Lee, Daniel, Jingoo,
Jacopo is travelling until November 1st and won't be able to test this
again before this date. Do you think you can pick it up and in case
anything's broken on SH, we can fix it after v5.5-rc1, so that it
doesn't miss another merge window?
Bart
[PATCH v7 0/9] backlight: gpio: simplify the driver
From: Bartosz Golaszewski
While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.
First two patches contain minor fixes. Third patch makes the driver
explicitly drive the GPIO line. Fourth patch adds all necessary data
structures to ecovec24. Patch 5/9 unifies much of the code for both
pdata and non-pdata cases. Patches 6-7/9 remove unused platform data
fields. Last two patches contain additional improvements for the GPIO
backlight driver while we're already modifying it.
I don't have access to this HW but hopefully this works. Only compile
tested.
[1] https://lkml.org/lkml/2019/6/25/900
v1 -> v2:
- rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
- added additional two patches with minor improvements
v2 -> v3:
- in patch 7/7: used initializers to set values for pdata and dev local vars
v3 -> v4:
- rebased on top of v5.4-rc1
- removed changes that are no longer relevant after commit ec665b756e6f
("backlight: gpio-backlight: Correct initial power state handling")
- added patch 7/7
v4 -> v5:
- in patch 7/7: added a comment replacing the name of the function being
pulled into probe()
v5 -> v6:
- added a patch making the driver explicitly set the direction of the GPIO
to output
- added a patch removing a redundant newline
v6 -> v7:
- renamed the function calculating the new GPIO value for status update
- collected more tags
Bartosz Golaszewski (9):
backlight: gpio: remove unneeded include
backlight: gpio: remove stray newline
backlight: gpio: explicitly set the direction of the GPIO
sh: ecovec24: add additional properties to the backlight device
backlight: gpio: simplify the platform data handling
sh: ecovec24: don't set unused fields in platform data
backlight: gpio: remove unused fields from platform data
backlight: gpio: use a helper variable for >dev
backlight: gpio: pull gpio_backlight_initial_power_state() into probe
arch/sh/boards/mach-ecovec24/setup.c | 33 +++--
drivers/video/backlight/gpio_backlight.c | 128 +++
include/linux/platform_data/gpio_backlight.h | 3 -
3 files changed, 69 insertions(+), 95 deletions(-)
--
2.23.0
[PATCH v7 5/9] backlight: gpio: simplify the platform data handling
From: Bartosz Golaszewski
Now that the last user of platform data (sh ecovec24) defines a proper
GPIO lookup and sets the 'default-on' device property, we can drop the
platform_data-specific GPIO handling and unify a big chunk of code.
The only field used from the platform data is now the fbdev pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 62 +---
1 file changed, 11 insertions(+), 51 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 52f17c9ca1c3..ddc7d3b288b7 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -6,7 +6,6 @@
#include
#include
#include
-#include /* Only for legacy support */
#include
#include
#include
@@ -61,28 +60,6 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_probe_dt(struct platform_device *pdev,
- struct gpio_backlight *gbl)
-{
- struct device *dev = >dev;
- int ret;
-
- gbl->def_value = device_property_read_bool(dev, "default-on");
-
- gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
- if (IS_ERR(gbl->gpiod)) {
- ret = PTR_ERR(gbl->gpiod);
-
- if (ret != -EPROBE_DEFER) {
- dev_err(dev,
- "Error: The gpios parameter is missing or
invalid.\n");
- }
- return ret;
- }
-
- return 0;
-}
-
static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
{
struct device_node *node = gbl->dev->of_node;
@@ -114,35 +91,18 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
gbl->dev = >dev;
- if (pdev->dev.fwnode) {
- ret = gpio_backlight_probe_dt(pdev, gbl);
- if (ret)
- return ret;
- } else if (pdata) {
- /*
-* Legacy platform data GPIO retrieveal. Do not expand
-* the use of this code path, currently only used by one
-* SH board.
-*/
- unsigned long flags = GPIOF_DIR_OUT;
-
+ if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = pdata->def_value;
- flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
-
- ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
- pdata ? pdata->name : "backlight");
- if (ret < 0) {
- dev_err(>dev, "unable to request GPIO\n");
- return ret;
- }
- gbl->gpiod = gpio_to_desc(pdata->gpio);
- if (!gbl->gpiod)
- return -EINVAL;
- } else {
- dev_err(>dev,
- "failed to find platform data or device tree node.\n");
- return -ENODEV;
+
+ gbl->def_value = device_property_read_bool(>dev, "default-on");
+
+ gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ if (IS_ERR(gbl->gpiod)) {
+ ret = PTR_ERR(gbl->gpiod);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev,
+ "Error: The gpios parameter is missing or
invalid.\n");
+ return ret;
}
memset(, 0, sizeof(props));
--
2.23.0
[PATCH v7 1/9] backlight: gpio: remove unneeded include
From: Bartosz Golaszewski We no longer use any symbols from of_gpio.h. Remove this include. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Reviewed-by: Daniel Thompson Reviewed-by: Andy Shevchenko --- drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 18e053e4716c..7e1990199fae 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include -- 2.23.0
[PATCH v7 6/9] sh: ecovec24: don't set unused fields in platform data
From: Bartosz Golaszewski
Platform data fields other than fbdev are no longer used by the
backlight driver. Remove them.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index aaa8ea62636f..dd427bac5cde 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -386,9 +386,6 @@ static struct property_entry gpio_backlight_props[] = {
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
- .gpio = GPIO_PTR1,
- .def_value = 1,
- .name = "backlight",
};
static const struct platform_device_info gpio_backlight_device_info = {
--
2.23.0
[PATCH v7 4/9] sh: ecovec24: add additional properties to the backlight device
From: Bartosz Golaszewski
Add a GPIO lookup entry and a device property for GPIO backlight to the
board file. Tie them to the platform device which is now registered using
platform_device_register_full() because of the properties. These changes
are inactive now but will be used once the gpio backlight driver is
modified.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 30 +++-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index acaa97459531..aaa8ea62636f 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -371,6 +371,19 @@ static struct platform_device lcdc_device = {
},
};
+static struct gpiod_lookup_table gpio_backlight_lookup = {
+ .dev_id = "gpio-backlight.0",
+ .table = {
+ GPIO_LOOKUP("sh7724_pfc", GPIO_PTR1, NULL, GPIO_ACTIVE_HIGH),
+ { }
+ },
+};
+
+static struct property_entry gpio_backlight_props[] = {
+ PROPERTY_ENTRY_BOOL("default-on"),
+ { }
+};
+
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
.gpio = GPIO_PTR1,
@@ -378,13 +391,15 @@ static struct gpio_backlight_platform_data
gpio_backlight_data = {
.name = "backlight",
};
-static struct platform_device gpio_backlight_device = {
+static const struct platform_device_info gpio_backlight_device_info = {
.name = "gpio-backlight",
- .dev = {
- .platform_data = _backlight_data,
- },
+ .data = _backlight_data,
+ .size_data = sizeof(gpio_backlight_data),
+ .properties = gpio_backlight_props,
};
+static struct platform_device *gpio_backlight_device;
+
/* CEU0 */
static struct ceu_platform_data ceu0_pdata = {
.num_subdevs= 2,
@@ -1006,7 +1021,6 @@ static struct platform_device *ecovec_devices[]
__initdata = {
_common_device,
_device,
_device,
- _backlight_device,
_device,
_power,
#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
@@ -1462,6 +1476,12 @@ static int __init arch_setup(void)
#endif
#endif
+ gpiod_add_lookup_table(_backlight_lookup);
+ gpio_backlight_device = platform_device_register_full(
+ _backlight_device_info);
+ if (IS_ERR(gpio_backlight_device))
+ return PTR_ERR(gpio_backlight_device);
+
return platform_add_devices(ecovec_devices,
ARRAY_SIZE(ecovec_devices));
}
--
2.23.0
Re: [PATCH v6 3/9] backlight: gpio: explicitly set the direction of the GPIO
pon., 21 paź 2019 o 12:45 Daniel Thompson
napisał(a):
>
> On Sat, Oct 19, 2019 at 10:35:50AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski
> >
> > The GPIO backlight driver currently requests the line 'as is', without
> > acively setting its direction. This can lead to problems: if the line
> > is in input mode by default, we won't be able to drive it later when
> > updating the status and also reading its initial value doesn't make
> > sense for backlight setting.
> >
> > Request the line 'as is' initially, so that we can read its value
> > without affecting it but then change the direction to output explicitly
> > when setting the initial brightness.
> >
> > Also: check the current direction and only read the value if it's output.
> >
> > Signed-off-by: Bartosz Golaszewski
>
> Intent looks good to me but...
>
> > ---
> > drivers/video/backlight/gpio_backlight.c | 23 ++-
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/video/backlight/gpio_backlight.c
> > b/drivers/video/backlight/gpio_backlight.c
> > index 3955b513f2f8..a36ac3a45b81 100644
> > --- a/drivers/video/backlight/gpio_backlight.c
> > +++ b/drivers/video/backlight/gpio_backlight.c
> > @@ -25,9 +25,8 @@ struct gpio_backlight {
> > int def_value;
> > };
> >
> > -static int gpio_backlight_update_status(struct backlight_device *bl)
> > +static int gpio_backlight_get_curr_brightness(struct backlight_device *bl)
>
> This function does not get the current brightness (e.g. what the
> hardware is currently doing). Given we've just nuked the function that
> *did* get the current brightness from the hardware this isn't an
> acceptable name.
>
> Would like something like calc_brightness() or get_next_brightness().
>
Fair enough, the latter sounds good in this case.
Bart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 4/9] sh: ecovec24: add additional properties to the backlight device
From: Bartosz Golaszewski
Add a GPIO lookup entry and a device property for GPIO backlight to the
board file. Tie them to the platform device which is now registered using
platform_device_register_full() because of the properties. These changes
are inactive now but will be used once the gpio backlight driver is
modified.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 30 +++-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index acaa97459531..aaa8ea62636f 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -371,6 +371,19 @@ static struct platform_device lcdc_device = {
},
};
+static struct gpiod_lookup_table gpio_backlight_lookup = {
+ .dev_id = "gpio-backlight.0",
+ .table = {
+ GPIO_LOOKUP("sh7724_pfc", GPIO_PTR1, NULL, GPIO_ACTIVE_HIGH),
+ { }
+ },
+};
+
+static struct property_entry gpio_backlight_props[] = {
+ PROPERTY_ENTRY_BOOL("default-on"),
+ { }
+};
+
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
.gpio = GPIO_PTR1,
@@ -378,13 +391,15 @@ static struct gpio_backlight_platform_data
gpio_backlight_data = {
.name = "backlight",
};
-static struct platform_device gpio_backlight_device = {
+static const struct platform_device_info gpio_backlight_device_info = {
.name = "gpio-backlight",
- .dev = {
- .platform_data = _backlight_data,
- },
+ .data = _backlight_data,
+ .size_data = sizeof(gpio_backlight_data),
+ .properties = gpio_backlight_props,
};
+static struct platform_device *gpio_backlight_device;
+
/* CEU0 */
static struct ceu_platform_data ceu0_pdata = {
.num_subdevs= 2,
@@ -1006,7 +1021,6 @@ static struct platform_device *ecovec_devices[]
__initdata = {
_common_device,
_device,
_device,
- _backlight_device,
_device,
_power,
#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
@@ -1462,6 +1476,12 @@ static int __init arch_setup(void)
#endif
#endif
+ gpiod_add_lookup_table(_backlight_lookup);
+ gpio_backlight_device = platform_device_register_full(
+ _backlight_device_info);
+ if (IS_ERR(gpio_backlight_device))
+ return PTR_ERR(gpio_backlight_device);
+
return platform_add_devices(ecovec_devices,
ARRAY_SIZE(ecovec_devices));
}
--
2.23.0
[PATCH v6 3/9] backlight: gpio: explicitly set the direction of the GPIO
From: Bartosz Golaszewski
The GPIO backlight driver currently requests the line 'as is', without
acively setting its direction. This can lead to problems: if the line
is in input mode by default, we won't be able to drive it later when
updating the status and also reading its initial value doesn't make
sense for backlight setting.
Request the line 'as is' initially, so that we can read its value
without affecting it but then change the direction to output explicitly
when setting the initial brightness.
Also: check the current direction and only read the value if it's output.
Signed-off-by: Bartosz Golaszewski
---
drivers/video/backlight/gpio_backlight.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 3955b513f2f8..a36ac3a45b81 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -25,9 +25,8 @@ struct gpio_backlight {
int def_value;
};
-static int gpio_backlight_update_status(struct backlight_device *bl)
+static int gpio_backlight_get_curr_brightness(struct backlight_device *bl)
{
- struct gpio_backlight *gbl = bl_get_data(bl);
int brightness = bl->props.brightness;
if (bl->props.power != FB_BLANK_UNBLANK ||
@@ -35,6 +34,14 @@ static int gpio_backlight_update_status(struct
backlight_device *bl)
bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
brightness = 0;
+ return brightness;
+}
+
+static int gpio_backlight_update_status(struct backlight_device *bl)
+{
+ struct gpio_backlight *gbl = bl_get_data(bl);
+ int brightness = gpio_backlight_get_curr_brightness(bl);
+
gpiod_set_value_cansleep(gbl->gpiod, brightness);
return 0;
@@ -85,7 +92,8 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
/* if the enable GPIO is disabled, do not enable the backlight */
- if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
+ if (gpiod_get_direction(gbl->gpiod) == 0 &&
+ gpiod_get_value_cansleep(gbl->gpiod) == 0)
return FB_BLANK_POWERDOWN;
return FB_BLANK_UNBLANK;
@@ -98,7 +106,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
- int ret;
+ int ret, init_brightness;
gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
@@ -151,7 +159,12 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
bl->props.power = gpio_backlight_initial_power_state(gbl);
bl->props.brightness = 1;
- backlight_update_status(bl);
+ init_brightness = gpio_backlight_get_curr_brightness(bl);
+ ret = gpiod_direction_output(gbl->gpiod, init_brightness);
+ if (ret) {
+ dev_err(>dev, "failed to set initial brightness\n");
+ return ret;
+ }
platform_set_drvdata(pdev, bl);
return 0;
--
2.23.0
[PATCH v6 0/9] backlight: gpio: simplify the driver
From: Bartosz Golaszewski
While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.
First two patches contain minor fixes. Third patch makes the driver
explicitly drive the GPIO line. Fourth patch adds all necessary data
structures to ecovec24. Patch 5/9 unifies much of the code for both
pdata and non-pdata cases. Patches 6-7/9 remove unused platform data
fields. Last two patches contain additional improvements for the GPIO
backlight driver while we're already modifying it.
I don't have access to this HW but hopefully this works. Only compile
tested.
[1] https://lkml.org/lkml/2019/6/25/900
v1 -> v2:
- rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
- added additional two patches with minor improvements
v2 -> v3:
- in patch 7/7: used initializers to set values for pdata and dev local vars
v3 -> v4:
- rebased on top of v5.4-rc1
- removed changes that are no longer relevant after commit ec665b756e6f
("backlight: gpio-backlight: Correct initial power state handling")
- added patch 7/7
v4 -> v5:
- in patch 7/7: added a comment replacing the name of the function being
pulled into probe()
v5 -> v6:
- added a patch making the driver explicitly set the direction of the GPIO
to output
- added a patch removing a redundant newline
Bartosz Golaszewski (9):
backlight: gpio: remove unneeded include
backlight: gpio: remove stray newline
backlight: gpio: explicitly set the direction of the GPIO
sh: ecovec24: add additional properties to the backlight device
backlight: gpio: simplify the platform data handling
sh: ecovec24: don't set unused fields in platform data
backlight: gpio: remove unused fields from platform data
backlight: gpio: use a helper variable for >dev
backlight: gpio: pull gpio_backlight_initial_power_state() into probe
arch/sh/boards/mach-ecovec24/setup.c | 33 +++--
drivers/video/backlight/gpio_backlight.c | 128 +++
include/linux/platform_data/gpio_backlight.h | 3 -
3 files changed, 69 insertions(+), 95 deletions(-)
--
2.23.0
[PATCH v6 2/9] backlight: gpio: remove stray newline
From: Bartosz Golaszewski
Remove a double newline from the driver.
Signed-off-by: Bartosz Golaszewski
---
drivers/video/backlight/gpio_backlight.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 7e1990199fae..3955b513f2f8 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -91,7 +91,6 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
return FB_BLANK_UNBLANK;
}
-
static int gpio_backlight_probe(struct platform_device *pdev)
{
struct gpio_backlight_platform_data *pdata =
--
2.23.0
[PATCH v6 1/9] backlight: gpio: remove unneeded include
From: Bartosz Golaszewski We no longer use any symbols from of_gpio.h. Remove this include. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Reviewed-by: Daniel Thompson Reviewed-by: Andy Shevchenko --- drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 18e053e4716c..7e1990199fae 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include -- 2.23.0
[PATCH v6 9/9] backlight: gpio: pull gpio_backlight_initial_power_state() into probe
From: Bartosz Golaszewski
The probe function in the gpio-backlight driver is quite short. If we
pull gpio_backlight_initial_power_state() into probe we can drop two
more fields from struct gpio_backlight and shrink the driver code.
Signed-off-by: Bartosz Golaszewski
Acked-by: Daniel Thompson
---
drivers/video/backlight/gpio_backlight.c | 38 +---
1 file changed, 14 insertions(+), 24 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 7b411f6ee15a..4336db6bf6b5 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -17,11 +17,8 @@
#include
struct gpio_backlight {
- struct device *dev;
struct device *fbdev;
-
struct gpio_desc *gpiod;
- int def_value;
};
static int gpio_backlight_get_curr_brightness(struct backlight_device *bl)
@@ -60,41 +57,24 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
-{
- struct device_node *node = gbl->dev->of_node;
-
- /* Not booted with device tree or no phandle link to the node */
- if (!node || !node->phandle)
- return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
-
- /* if the enable GPIO is disabled, do not enable the backlight */
- if (gpiod_get_direction(gbl->gpiod) == 0 &&
- gpiod_get_value_cansleep(gbl->gpiod) == 0)
- return FB_BLANK_POWERDOWN;
-
- return FB_BLANK_UNBLANK;
-}
-
static int gpio_backlight_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
+ struct device_node *of_node = dev->of_node;
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
- int ret, init_brightness;
+ int ret, init_brightness, def_value;
gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = dev;
-
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(dev, "default-on");
+ def_value = device_property_read_bool(dev, "default-on");
gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
@@ -115,7 +95,17 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
return PTR_ERR(bl);
}
- bl->props.power = gpio_backlight_initial_power_state(gbl);
+ /* Set the initial power state */
+ if (!of_node || !of_node->phandle)
+ /* Not booted with device tree or no phandle link to the node */
+ bl->props.power = def_value ? FB_BLANK_UNBLANK
+ : FB_BLANK_POWERDOWN;
+ else if (gpiod_get_direction(gbl->gpiod) == 0 &&
+gpiod_get_value_cansleep(gbl->gpiod) == 0)
+ bl->props.power = FB_BLANK_POWERDOWN;
+ else
+ bl->props.power = FB_BLANK_UNBLANK;
+
bl->props.brightness = 1;
init_brightness = gpio_backlight_get_curr_brightness(bl);
--
2.23.0
[PATCH v6 8/9] backlight: gpio: use a helper variable for >dev
From: Bartosz Golaszewski
Instead of dereferencing pdev each time, use a helper variable for
the associated device pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index c829b82f5372..7b411f6ee15a 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -78,29 +78,29 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
static int gpio_backlight_probe(struct platform_device *pdev)
{
- struct gpio_backlight_platform_data *pdata =
- dev_get_platdata(>dev);
+ struct device *dev = >dev;
+ struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
int ret, init_brightness;
- gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
+ gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = >dev;
+ gbl->dev = dev;
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(>dev, "default-on");
+ gbl->def_value = device_property_read_bool(dev, "default-on");
- gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
ret = PTR_ERR(gbl->gpiod);
if (ret != -EPROBE_DEFER)
- dev_err(>dev,
+ dev_err(dev,
"Error: The gpios parameter is missing or
invalid.\n");
return ret;
}
@@ -108,11 +108,10 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
memset(, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = 1;
- bl = devm_backlight_device_register(>dev, dev_name(>dev),
- >dev, gbl, _backlight_ops,
- );
+ bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
+ _backlight_ops, );
if (IS_ERR(bl)) {
- dev_err(>dev, "failed to register backlight\n");
+ dev_err(dev, "failed to register backlight\n");
return PTR_ERR(bl);
}
@@ -122,7 +121,7 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
init_brightness = gpio_backlight_get_curr_brightness(bl);
ret = gpiod_direction_output(gbl->gpiod, init_brightness);
if (ret) {
- dev_err(>dev, "failed to set initial brightness\n");
+ dev_err(dev, "failed to set initial brightness\n");
return ret;
}
--
2.23.0
[PATCH v6 7/9] backlight: gpio: remove unused fields from platform data
From: Bartosz Golaszewski
Remove the platform data fields that nobody uses.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
---
include/linux/platform_data/gpio_backlight.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/include/linux/platform_data/gpio_backlight.h
b/include/linux/platform_data/gpio_backlight.h
index 34179d600360..1a8b5b1946fe 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h
@@ -9,9 +9,6 @@ struct device;
struct gpio_backlight_platform_data {
struct device *fbdev;
- int gpio;
- int def_value;
- const char *name;
};
#endif
--
2.23.0
[PATCH v6 6/9] sh: ecovec24: don't set unused fields in platform data
From: Bartosz Golaszewski
Platform data fields other than fbdev are no longer used by the
backlight driver. Remove them.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index aaa8ea62636f..dd427bac5cde 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -386,9 +386,6 @@ static struct property_entry gpio_backlight_props[] = {
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
- .gpio = GPIO_PTR1,
- .def_value = 1,
- .name = "backlight",
};
static const struct platform_device_info gpio_backlight_device_info = {
--
2.23.0
[PATCH v6 5/9] backlight: gpio: simplify the platform data handling
From: Bartosz Golaszewski
Now that the last user of platform data (sh ecovec24) defines a proper
GPIO lookup and sets the 'default-on' device property, we can drop the
platform_data-specific GPIO handling and unify a big chunk of code.
The only field used from the platform data is now the fbdev pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 62 +---
1 file changed, 11 insertions(+), 51 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index a36ac3a45b81..c829b82f5372 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -6,7 +6,6 @@
#include
#include
#include
-#include /* Only for legacy support */
#include
#include
#include
@@ -61,28 +60,6 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_probe_dt(struct platform_device *pdev,
- struct gpio_backlight *gbl)
-{
- struct device *dev = >dev;
- int ret;
-
- gbl->def_value = device_property_read_bool(dev, "default-on");
-
- gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
- if (IS_ERR(gbl->gpiod)) {
- ret = PTR_ERR(gbl->gpiod);
-
- if (ret != -EPROBE_DEFER) {
- dev_err(dev,
- "Error: The gpios parameter is missing or
invalid.\n");
- }
- return ret;
- }
-
- return 0;
-}
-
static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
{
struct device_node *node = gbl->dev->of_node;
@@ -114,35 +91,18 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
gbl->dev = >dev;
- if (pdev->dev.fwnode) {
- ret = gpio_backlight_probe_dt(pdev, gbl);
- if (ret)
- return ret;
- } else if (pdata) {
- /*
-* Legacy platform data GPIO retrieveal. Do not expand
-* the use of this code path, currently only used by one
-* SH board.
-*/
- unsigned long flags = GPIOF_DIR_OUT;
-
+ if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = pdata->def_value;
- flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
-
- ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
- pdata ? pdata->name : "backlight");
- if (ret < 0) {
- dev_err(>dev, "unable to request GPIO\n");
- return ret;
- }
- gbl->gpiod = gpio_to_desc(pdata->gpio);
- if (!gbl->gpiod)
- return -EINVAL;
- } else {
- dev_err(>dev,
- "failed to find platform data or device tree node.\n");
- return -ENODEV;
+
+ gbl->def_value = device_property_read_bool(>dev, "default-on");
+
+ gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ if (IS_ERR(gbl->gpiod)) {
+ ret = PTR_ERR(gbl->gpiod);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev,
+ "Error: The gpios parameter is missing or
invalid.\n");
+ return ret;
}
memset(, 0, sizeof(props));
--
2.23.0
Re: [PATCH v5 0/7] backlight: gpio: simplify the driver
pt., 18 paź 2019 o 17:02 Jacopo Mondi napisał(a): > > Hi, > > On Thu, Oct 17, 2019 at 08:25:50AM +0100, Lee Jones wrote: > > On Wed, 16 Oct 2019, Jacopo Mondi wrote: > > > > > Hi, sorry for not having replied earlier > > > > > > On Wed, Oct 16, 2019 at 02:56:57PM +0200, Linus Walleij wrote: > > > > On Mon, Oct 14, 2019 at 10:12 AM Lee Jones wrote: > > > > > > > > > > arch/sh/boards/mach-ecovec24/setup.c | 33 -- > > > > > > > > > > I guess we're just waiting for the SH Acks now? > > > > > > > > The one maintainer with this board is probably overloaded. > > > > > > > > I would say just apply it, it can't hold back the entire series. > > > > > > I've been able to resurect the Ecovec, and I've also been given a copy > > > of its schematics file a few weeks ago. > > > > > > It's in my TODO list to test this series but I didn't manage to find > > > time. If I pinky promise I get back to you before end of the week, > > > could you wait for me ? :) > > Finally had some time to spend on this. > > As I've reported to Bartosz, this version does not work on Ecovec out > of the box, as the GPIO line connected to the backlight needs to be > configured to work in output mode before registering the backlight > device. > > With this simple change: > > $ git diff > diff --git a/arch/sh/boards/mach-ecovec24/setup.c > b/arch/sh/boards/mach-ecovec24/setup.c > index dd427bac5cde..eec6e805c3ed 100644 > --- a/arch/sh/boards/mach-ecovec24/setup.c > +++ b/arch/sh/boards/mach-ecovec24/setup.c > @@ -1473,6 +1473,7 @@ static int __init arch_setup(void) > #endif > #endif > > + gpio_direction_output(GPIO_PTR1, 1); This is a hack actually. The problem lies with the gpio backlight driver - it should really set the mode to output, not leave it as is. If there's no good reason to keep it as it is now, I'll add another patch to the series that moves the call to devm_gpiod_get() to where we've already determined the initial value in probe() and pass an appropriate GPIOD_OUT_HIGH/LOW flag. Bart > gpiod_add_lookup_table(_backlight_lookup); > gpio_backlight_device = platform_device_register_full( > _backlight_device_info); > > I can now control the gpio through the backlight interface. > > So please add this bit on top of next iteration and add my: > Tested-by: Jacopo Mondi > > Thanks and sorry for the long time it took! > > > > > Yes, no problem. > > > > -- > > Lee Jones [李琼斯] > > Linaro Services Technical Lead > > Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v5 0/7] backlight: gpio: simplify the driver
pon., 14 paź 2019 o 10:12 Lee Jones napisał(a):
>
> On Mon, 07 Oct 2019, Bartosz Golaszewski wrote:
>
> > From: Bartosz Golaszewski
> >
> > While working on my other series related to gpio-backlight[1] I noticed
> > that we could simplify the driver if we made the only user of platform
> > data use GPIO lookups and device properties. This series tries to do
> > that.
> >
> > The first patch adds all necessary data structures to ecovec24. Patch
> > 2/7 unifies much of the code for both pdata and non-pdata cases. Patches
> > 3-4/7 remove unused platform data fields. Last three patches contain
> > additional improvements for the GPIO backlight driver while we're already
> > modifying it.
> >
> > I don't have access to this HW but hopefully this works. Only compile
> > tested.
> >
> > [1] https://lkml.org/lkml/2019/6/25/900
> >
> > v1 -> v2:
> > - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
> > - added additional two patches with minor improvements
> >
> > v2 -> v3:
> > - in patch 7/7: used initializers to set values for pdata and dev local vars
> >
> > v3 -> v4:
> > - rebased on top of v5.4-rc1
> > - removed changes that are no longer relevant after commit ec665b756e6f
> > ("backlight: gpio-backlight: Correct initial power state handling")
> > - added patch 7/7
> >
> > v4 ->V5:
> > - in patch 7/7: added a comment replacing the name of the function being
> > pulled into probe()
> >
> > Bartosz Golaszewski (7):
> > backlight: gpio: remove unneeded include
> > sh: ecovec24: add additional properties to the backlight device
> > backlight: gpio: simplify the platform data handling
> > sh: ecovec24: don't set unused fields in platform data
> > backlight: gpio: remove unused fields from platform data
> > backlight: gpio: use a helper variable for >dev
> > backlight: gpio: pull gpio_backlight_initial_power_state() into probe
> >
> > arch/sh/boards/mach-ecovec24/setup.c | 33 --
>
> I guess we're just waiting for the SH Acks now?
>
We've been waiting for them for a couple months now - the sh patches
haven't changed since v1...
Rich, Yoshinori - could you ack this so that it can go in for v5.5?
Thanks,
Bartosz
> > drivers/video/backlight/gpio_backlight.c | 108 +--
> > include/linux/platform_data/gpio_backlight.h | 3 -
> > 3 files changed, 53 insertions(+), 91 deletions(-)
> >
>
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
[PATCH v5 7/7] backlight: gpio: pull gpio_backlight_initial_power_state() into probe
From: Bartosz Golaszewski
The probe function in the gpio-backlight driver is quite short. If we
pull gpio_backlight_initial_power_state() into probe we can drop two
more fields from struct gpio_backlight and shrink the driver code.
Signed-off-by: Bartosz Golaszewski
---
drivers/video/backlight/gpio_backlight.c | 37 +---
1 file changed, 13 insertions(+), 24 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 6247687b6330..407d4eaafc5c 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -17,11 +17,8 @@
#include
struct gpio_backlight {
- struct device *dev;
struct device *fbdev;
-
struct gpio_desc *gpiod;
- int def_value;
};
static int gpio_backlight_update_status(struct backlight_device *bl)
@@ -53,41 +50,24 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
-{
- struct device_node *node = gbl->dev->of_node;
-
- /* Not booted with device tree or no phandle link to the node */
- if (!node || !node->phandle)
- return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
-
- /* if the enable GPIO is disabled, do not enable the backlight */
- if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
- return FB_BLANK_POWERDOWN;
-
- return FB_BLANK_UNBLANK;
-}
-
-
static int gpio_backlight_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
+ struct device_node *of_node = dev->of_node;
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
- int ret;
+ int ret, def_value;
gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = dev;
-
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(dev, "default-on");
+ def_value = device_property_read_bool(dev, "default-on");
gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
@@ -109,7 +89,16 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
return PTR_ERR(bl);
}
- bl->props.power = gpio_backlight_initial_power_state(gbl);
+ /* Set the initial power state */
+ if (!of_node || !of_node->phandle)
+ /* Not booted with device tree or no phandle link to the node */
+ bl->props.power = def_value ? FB_BLANK_UNBLANK
+ : FB_BLANK_POWERDOWN;
+ else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
+ bl->props.power = FB_BLANK_POWERDOWN;
+ else
+ bl->props.power = FB_BLANK_UNBLANK;
+
bl->props.brightness = 1;
backlight_update_status(bl);
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 3/7] backlight: gpio: simplify the platform data handling
From: Bartosz Golaszewski
Now that the last user of platform data (sh ecovec24) defines a proper
GPIO lookup and sets the 'default-on' device property, we can drop the
platform_data-specific GPIO handling and unify a big chunk of code.
The only field used from the platform data is now the fbdev pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 62 +---
1 file changed, 11 insertions(+), 51 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 7e1990199fae..20c5311c7ed2 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -6,7 +6,6 @@
#include
#include
#include
-#include /* Only for legacy support */
#include
#include
#include
@@ -54,28 +53,6 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_probe_dt(struct platform_device *pdev,
- struct gpio_backlight *gbl)
-{
- struct device *dev = >dev;
- int ret;
-
- gbl->def_value = device_property_read_bool(dev, "default-on");
-
- gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
- if (IS_ERR(gbl->gpiod)) {
- ret = PTR_ERR(gbl->gpiod);
-
- if (ret != -EPROBE_DEFER) {
- dev_err(dev,
- "Error: The gpios parameter is missing or
invalid.\n");
- }
- return ret;
- }
-
- return 0;
-}
-
static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
{
struct device_node *node = gbl->dev->of_node;
@@ -107,35 +84,18 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
gbl->dev = >dev;
- if (pdev->dev.fwnode) {
- ret = gpio_backlight_probe_dt(pdev, gbl);
- if (ret)
- return ret;
- } else if (pdata) {
- /*
-* Legacy platform data GPIO retrieveal. Do not expand
-* the use of this code path, currently only used by one
-* SH board.
-*/
- unsigned long flags = GPIOF_DIR_OUT;
-
+ if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = pdata->def_value;
- flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
-
- ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
- pdata ? pdata->name : "backlight");
- if (ret < 0) {
- dev_err(>dev, "unable to request GPIO\n");
- return ret;
- }
- gbl->gpiod = gpio_to_desc(pdata->gpio);
- if (!gbl->gpiod)
- return -EINVAL;
- } else {
- dev_err(>dev,
- "failed to find platform data or device tree node.\n");
- return -ENODEV;
+
+ gbl->def_value = device_property_read_bool(>dev, "default-on");
+
+ gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ if (IS_ERR(gbl->gpiod)) {
+ ret = PTR_ERR(gbl->gpiod);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev,
+ "Error: The gpios parameter is missing or
invalid.\n");
+ return ret;
}
memset(, 0, sizeof(props));
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 4/7] sh: ecovec24: don't set unused fields in platform data
From: Bartosz Golaszewski
Platform data fields other than fbdev are no longer used by the
backlight driver. Remove them.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index aaa8ea62636f..dd427bac5cde 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -386,9 +386,6 @@ static struct property_entry gpio_backlight_props[] = {
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
- .gpio = GPIO_PTR1,
- .def_value = 1,
- .name = "backlight",
};
static const struct platform_device_info gpio_backlight_device_info = {
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 5/7] backlight: gpio: remove unused fields from platform data
From: Bartosz Golaszewski
Remove the platform data fields that nobody uses.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
---
include/linux/platform_data/gpio_backlight.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/include/linux/platform_data/gpio_backlight.h
b/include/linux/platform_data/gpio_backlight.h
index 34179d600360..1a8b5b1946fe 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h
@@ -9,9 +9,6 @@ struct device;
struct gpio_backlight_platform_data {
struct device *fbdev;
- int gpio;
- int def_value;
- const char *name;
};
#endif
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 6/7] backlight: gpio: use a helper variable for >dev
From: Bartosz Golaszewski
Instead of dereferencing pdev each time, use a helper variable for
the associated device pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 20c5311c7ed2..6247687b6330 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -71,25 +71,25 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
static int gpio_backlight_probe(struct platform_device *pdev)
{
- struct gpio_backlight_platform_data *pdata =
- dev_get_platdata(>dev);
+ struct device *dev = >dev;
+ struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
int ret;
- gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
+ gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = >dev;
+ gbl->dev = dev;
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(>dev, "default-on");
+ gbl->def_value = device_property_read_bool(dev, "default-on");
- gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
ret = PTR_ERR(gbl->gpiod);
if (ret != -EPROBE_DEFER)
@@ -101,11 +101,11 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
memset(, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = 1;
- bl = devm_backlight_device_register(>dev, dev_name(>dev),
- >dev, gbl, _backlight_ops,
+ bl = devm_backlight_device_register(dev, dev_name(dev),
+ dev, gbl, _backlight_ops,
);
if (IS_ERR(bl)) {
- dev_err(>dev, "failed to register backlight\n");
+ dev_err(dev, "failed to register backlight\n");
return PTR_ERR(bl);
}
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/7] sh: ecovec24: add additional properties to the backlight device
From: Bartosz Golaszewski
Add a GPIO lookup entry and a device property for GPIO backlight to the
board file. Tie them to the platform device which is now registered using
platform_device_register_full() because of the properties. These changes
are inactive now but will be used once the gpio backlight driver is
modified.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 30 +++-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index acaa97459531..aaa8ea62636f 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -371,6 +371,19 @@ static struct platform_device lcdc_device = {
},
};
+static struct gpiod_lookup_table gpio_backlight_lookup = {
+ .dev_id = "gpio-backlight.0",
+ .table = {
+ GPIO_LOOKUP("sh7724_pfc", GPIO_PTR1, NULL, GPIO_ACTIVE_HIGH),
+ { }
+ },
+};
+
+static struct property_entry gpio_backlight_props[] = {
+ PROPERTY_ENTRY_BOOL("default-on"),
+ { }
+};
+
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
.gpio = GPIO_PTR1,
@@ -378,13 +391,15 @@ static struct gpio_backlight_platform_data
gpio_backlight_data = {
.name = "backlight",
};
-static struct platform_device gpio_backlight_device = {
+static const struct platform_device_info gpio_backlight_device_info = {
.name = "gpio-backlight",
- .dev = {
- .platform_data = _backlight_data,
- },
+ .data = _backlight_data,
+ .size_data = sizeof(gpio_backlight_data),
+ .properties = gpio_backlight_props,
};
+static struct platform_device *gpio_backlight_device;
+
/* CEU0 */
static struct ceu_platform_data ceu0_pdata = {
.num_subdevs= 2,
@@ -1006,7 +1021,6 @@ static struct platform_device *ecovec_devices[]
__initdata = {
_common_device,
_device,
_device,
- _backlight_device,
_device,
_power,
#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
@@ -1462,6 +1476,12 @@ static int __init arch_setup(void)
#endif
#endif
+ gpiod_add_lookup_table(_backlight_lookup);
+ gpio_backlight_device = platform_device_register_full(
+ _backlight_device_info);
+ if (IS_ERR(gpio_backlight_device))
+ return PTR_ERR(gpio_backlight_device);
+
return platform_add_devices(ecovec_devices,
ARRAY_SIZE(ecovec_devices));
}
--
2.23.0
[PATCH v5 0/7] backlight: gpio: simplify the driver
From: Bartosz Golaszewski
While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.
The first patch adds all necessary data structures to ecovec24. Patch
2/7 unifies much of the code for both pdata and non-pdata cases. Patches
3-4/7 remove unused platform data fields. Last three patches contain
additional improvements for the GPIO backlight driver while we're already
modifying it.
I don't have access to this HW but hopefully this works. Only compile
tested.
[1] https://lkml.org/lkml/2019/6/25/900
v1 -> v2:
- rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
- added additional two patches with minor improvements
v2 -> v3:
- in patch 7/7: used initializers to set values for pdata and dev local vars
v3 -> v4:
- rebased on top of v5.4-rc1
- removed changes that are no longer relevant after commit ec665b756e6f
("backlight: gpio-backlight: Correct initial power state handling")
- added patch 7/7
v4 ->V5:
- in patch 7/7: added a comment replacing the name of the function being
pulled into probe()
Bartosz Golaszewski (7):
backlight: gpio: remove unneeded include
sh: ecovec24: add additional properties to the backlight device
backlight: gpio: simplify the platform data handling
sh: ecovec24: don't set unused fields in platform data
backlight: gpio: remove unused fields from platform data
backlight: gpio: use a helper variable for >dev
backlight: gpio: pull gpio_backlight_initial_power_state() into probe
arch/sh/boards/mach-ecovec24/setup.c | 33 --
drivers/video/backlight/gpio_backlight.c | 108 +--
include/linux/platform_data/gpio_backlight.h | 3 -
3 files changed, 53 insertions(+), 91 deletions(-)
--
2.23.0
[PATCH v5 1/7] backlight: gpio: remove unneeded include
From: Bartosz Golaszewski We no longer use any symbols from of_gpio.h. Remove this include. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Reviewed-by: Daniel Thompson Reviewed-by: Andy Shevchenko --- drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 18e053e4716c..7e1990199fae 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include -- 2.23.0
[PATCH v5 0/7] backlight: gpio: simplify the driver
From: Bartosz Golaszewski
While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.
The first patch adds all necessary data structures to ecovec24. Patch
2/7 unifies much of the code for both pdata and non-pdata cases. Patches
3-4/7 remove unused platform data fields. Last three patches contain
additional improvements for the GPIO backlight driver while we're already
modifying it.
I don't have access to this HW but hopefully this works. Only compile
tested.
[1] https://lkml.org/lkml/2019/6/25/900
v1 -> v2:
- rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
- added additional two patches with minor improvements
v2 -> v3:
- in patch 7/7: used initializers to set values for pdata and dev local vars
v3 -> v4:
- rebased on top of v5.4-rc1
- removed changes that are no longer relevant after commit ec665b756e6f
("backlight: gpio-backlight: Correct initial power state handling")
- added patch 7/7
v4 ->V5:
- in patch 7/7: added a comment replacing the name of the function being
pulled into probe()
Bartosz Golaszewski (7):
backlight: gpio: remove unneeded include
sh: ecovec24: add additional properties to the backlight device
backlight: gpio: simplify the platform data handling
sh: ecovec24: don't set unused fields in platform data
backlight: gpio: remove unused fields from platform data
backlight: gpio: use a helper variable for >dev
backlight: gpio: pull gpio_backlight_initial_power_state() into probe
arch/sh/boards/mach-ecovec24/setup.c | 33 --
drivers/video/backlight/gpio_backlight.c | 108 +--
include/linux/platform_data/gpio_backlight.h | 3 -
3 files changed, 53 insertions(+), 91 deletions(-)
--
2.23.0
Re: [PATCH v4 7/7] backlight: gpio: pull gpio_backlight_initial_power_state() into probe
śr., 2 paź 2019 o 16:40 Daniel Thompson napisał(a):
>
> On Wed, Oct 02, 2019 at 01:46:17PM +0200, Bartosz Golaszewski wrote:
> > śr., 2 paź 2019 o 12:33 Daniel Thompson
> > napisał(a):
> > >
> > > On Tue, Oct 01, 2019 at 02:58:37PM +0200, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski
> > > >
> > > > The probe function in the gpio-backlight driver is quite short. If we
> > > > pull gpio_backlight_initial_power_state() into probe we can drop two
> > > > more fields from struct gpio_backlight and shrink the driver code.
> > > >
> > > > Signed-off-by: Bartosz Golaszewski
> > > > ---
> > > > drivers/video/backlight/gpio_backlight.c | 36
> > > > 1 file changed, 12 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/video/backlight/gpio_backlight.c
> > > > b/drivers/video/backlight/gpio_backlight.c
> > > > index 6247687b6330..37ec184f0c5c 100644
> > > > --- a/drivers/video/backlight/gpio_backlight.c
> > > > +++ b/drivers/video/backlight/gpio_backlight.c
> > > > @@ -17,11 +17,8 @@
> > > > #include
> > > >
> > > > struct gpio_backlight {
> > > > - struct device *dev;
> > > > struct device *fbdev;
> > > > -
> > > > struct gpio_desc *gpiod;
> > > > - int def_value;
> > > > };
> > > >
> > > > static int gpio_backlight_update_status(struct backlight_device *bl)
> > > > @@ -53,41 +50,24 @@ static const struct backlight_ops
> > > > gpio_backlight_ops = {
> > > > .check_fb = gpio_backlight_check_fb,
> > > > };
> > > >
> > > > -static int gpio_backlight_initial_power_state(struct gpio_backlight
> > > > *gbl)
> > >
> > > I'm inclined to view deleting this function as removing a comment (e.g.
> > > the function name helps us to read the code)!
> > >
> >
> > Right, but why not just add a comment then?
>
> I guess you could add a comment but keeping it pulled out in a function
> makes it easier to compare against equivalent code in other drivers
> (such as pwm_bl).
>
The pwm driver seems to be the only one that has this function and
it's also much more complicated. Unless it's a hard NACK, I think that
pulling all initialization into probe looks better and shrinks the
driver visually.
Bart
>
> Daniel.
>
>
> > The probe function is 50
> > lines long, there's really no need to split it. This will get inlined
> > anyway too.
> >
> > Bart
> >
> > > Removing the variables from the context structure is good but why not
> > > just pass them to the function and let the compiler decided whether or
> > > not to inline.
> > >
> > >
> > > Daniel.
> > >
> > >
> > > > -{
> > > > - struct device_node *node = gbl->dev->of_node;
> > > > -
> > > > - /* Not booted with device tree or no phandle link to the node */
> > > > - if (!node || !node->phandle)
> > > > - return gbl->def_value ? FB_BLANK_UNBLANK :
> > > > FB_BLANK_POWERDOWN;
> > > > -
> > > > - /* if the enable GPIO is disabled, do not enable the backlight */
> > > > - if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> > > > - return FB_BLANK_POWERDOWN;
> > > > -
> > > > - return FB_BLANK_UNBLANK;
> > > > -}
> > > > -
> > > > -
> > > > static int gpio_backlight_probe(struct platform_device *pdev)
> > > > {
> > > > struct device *dev = >dev;
> > > > struct gpio_backlight_platform_data *pdata =
> > > > dev_get_platdata(dev);
> > > > + struct device_node *of_node = dev->of_node;
> > > > struct backlight_properties props;
> > > > struct backlight_device *bl;
> > > > struct gpio_backlight *gbl;
> > > > - int ret;
> > > > + int ret, def_value;
> > > >
> > > > gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
> > > > if (gbl == NULL)
> > > > return -ENOMEM;
> > > >
> > > > - gbl->dev = dev;
> > > > -
> > > > if (pdata)
> > > > gbl->fbdev = pdata->fbdev;
> &
Re: [PATCH v4 7/7] backlight: gpio: pull gpio_backlight_initial_power_state() into probe
śr., 2 paź 2019 o 12:33 Daniel Thompson napisał(a):
>
> On Tue, Oct 01, 2019 at 02:58:37PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski
> >
> > The probe function in the gpio-backlight driver is quite short. If we
> > pull gpio_backlight_initial_power_state() into probe we can drop two
> > more fields from struct gpio_backlight and shrink the driver code.
> >
> > Signed-off-by: Bartosz Golaszewski
> > ---
> > drivers/video/backlight/gpio_backlight.c | 36
> > 1 file changed, 12 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/video/backlight/gpio_backlight.c
> > b/drivers/video/backlight/gpio_backlight.c
> > index 6247687b6330..37ec184f0c5c 100644
> > --- a/drivers/video/backlight/gpio_backlight.c
> > +++ b/drivers/video/backlight/gpio_backlight.c
> > @@ -17,11 +17,8 @@
> > #include
> >
> > struct gpio_backlight {
> > - struct device *dev;
> > struct device *fbdev;
> > -
> > struct gpio_desc *gpiod;
> > - int def_value;
> > };
> >
> > static int gpio_backlight_update_status(struct backlight_device *bl)
> > @@ -53,41 +50,24 @@ static const struct backlight_ops gpio_backlight_ops = {
> > .check_fb = gpio_backlight_check_fb,
> > };
> >
> > -static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
>
> I'm inclined to view deleting this function as removing a comment (e.g.
> the function name helps us to read the code)!
>
Right, but why not just add a comment then? The probe function is 50
lines long, there's really no need to split it. This will get inlined
anyway too.
Bart
> Removing the variables from the context structure is good but why not
> just pass them to the function and let the compiler decided whether or
> not to inline.
>
>
> Daniel.
>
>
> > -{
> > - struct device_node *node = gbl->dev->of_node;
> > -
> > - /* Not booted with device tree or no phandle link to the node */
> > - if (!node || !node->phandle)
> > - return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
> > -
> > - /* if the enable GPIO is disabled, do not enable the backlight */
> > - if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> > - return FB_BLANK_POWERDOWN;
> > -
> > - return FB_BLANK_UNBLANK;
> > -}
> > -
> > -
> > static int gpio_backlight_probe(struct platform_device *pdev)
> > {
> > struct device *dev = >dev;
> > struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
> > + struct device_node *of_node = dev->of_node;
> > struct backlight_properties props;
> > struct backlight_device *bl;
> > struct gpio_backlight *gbl;
> > - int ret;
> > + int ret, def_value;
> >
> > gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
> > if (gbl == NULL)
> > return -ENOMEM;
> >
> > - gbl->dev = dev;
> > -
> > if (pdata)
> > gbl->fbdev = pdata->fbdev;
> >
> > - gbl->def_value = device_property_read_bool(dev, "default-on");
> > + def_value = device_property_read_bool(dev, "default-on");
> >
> > gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
> > if (IS_ERR(gbl->gpiod)) {
> > @@ -109,7 +89,15 @@ static int gpio_backlight_probe(struct platform_device
> > *pdev)
> > return PTR_ERR(bl);
> > }
> >
> > - bl->props.power = gpio_backlight_initial_power_state(gbl);
> > + /* Not booted with device tree or no phandle link to the node */
> > + if (!of_node || !of_node->phandle)
> > + bl->props.power = def_value ? FB_BLANK_UNBLANK
> > + : FB_BLANK_POWERDOWN;
> > + else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> > + bl->props.power = FB_BLANK_POWERDOWN;
> > + else
> > + bl->props.power = FB_BLANK_UNBLANK;
> > +
> > bl->props.brightness = 1;
> >
> > backlight_update_status(bl);
> > --
> > 2.23.0
> >
[PATCH v4 5/7] backlight: gpio: remove unused fields from platform data
From: Bartosz Golaszewski
Remove the platform data fields that nobody uses.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
---
include/linux/platform_data/gpio_backlight.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/include/linux/platform_data/gpio_backlight.h
b/include/linux/platform_data/gpio_backlight.h
index 34179d600360..1a8b5b1946fe 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h
@@ -9,9 +9,6 @@ struct device;
struct gpio_backlight_platform_data {
struct device *fbdev;
- int gpio;
- int def_value;
- const char *name;
};
#endif
--
2.23.0
[PATCH v4 1/7] backlight: gpio: remove unneeded include
From: Bartosz Golaszewski We no longer use any symbols from of_gpio.h. Remove this include. Signed-off-by: Bartosz Golaszewski --- drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 18e053e4716c..7e1990199fae 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include -- 2.23.0
[PATCH v4 7/7] backlight: gpio: pull gpio_backlight_initial_power_state() into probe
From: Bartosz Golaszewski
The probe function in the gpio-backlight driver is quite short. If we
pull gpio_backlight_initial_power_state() into probe we can drop two
more fields from struct gpio_backlight and shrink the driver code.
Signed-off-by: Bartosz Golaszewski
---
drivers/video/backlight/gpio_backlight.c | 36
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 6247687b6330..37ec184f0c5c 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -17,11 +17,8 @@
#include
struct gpio_backlight {
- struct device *dev;
struct device *fbdev;
-
struct gpio_desc *gpiod;
- int def_value;
};
static int gpio_backlight_update_status(struct backlight_device *bl)
@@ -53,41 +50,24 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
-{
- struct device_node *node = gbl->dev->of_node;
-
- /* Not booted with device tree or no phandle link to the node */
- if (!node || !node->phandle)
- return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
-
- /* if the enable GPIO is disabled, do not enable the backlight */
- if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
- return FB_BLANK_POWERDOWN;
-
- return FB_BLANK_UNBLANK;
-}
-
-
static int gpio_backlight_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
+ struct device_node *of_node = dev->of_node;
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
- int ret;
+ int ret, def_value;
gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = dev;
-
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(dev, "default-on");
+ def_value = device_property_read_bool(dev, "default-on");
gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
@@ -109,7 +89,15 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
return PTR_ERR(bl);
}
- bl->props.power = gpio_backlight_initial_power_state(gbl);
+ /* Not booted with device tree or no phandle link to the node */
+ if (!of_node || !of_node->phandle)
+ bl->props.power = def_value ? FB_BLANK_UNBLANK
+ : FB_BLANK_POWERDOWN;
+ else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
+ bl->props.power = FB_BLANK_POWERDOWN;
+ else
+ bl->props.power = FB_BLANK_UNBLANK;
+
bl->props.brightness = 1;
backlight_update_status(bl);
--
2.23.0
[PATCH v4 4/7] sh: ecovec24: don't set unused fields in platform data
From: Bartosz Golaszewski
Platform data fields other than fbdev are no longer used by the
backlight driver. Remove them.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index aaa8ea62636f..dd427bac5cde 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -386,9 +386,6 @@ static struct property_entry gpio_backlight_props[] = {
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
- .gpio = GPIO_PTR1,
- .def_value = 1,
- .name = "backlight",
};
static const struct platform_device_info gpio_backlight_device_info = {
--
2.23.0
[PATCH v4 3/7] backlight: gpio: simplify the platform data handling
From: Bartosz Golaszewski
Now that the last user of platform data (sh ecovec24) defines a proper
GPIO lookup and sets the 'default-on' device property, we can drop the
platform_data-specific GPIO handling and unify a big chunk of code.
The only field used from the platform data is now the fbdev pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 62 +---
1 file changed, 11 insertions(+), 51 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 7e1990199fae..20c5311c7ed2 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -6,7 +6,6 @@
#include
#include
#include
-#include /* Only for legacy support */
#include
#include
#include
@@ -54,28 +53,6 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_probe_dt(struct platform_device *pdev,
- struct gpio_backlight *gbl)
-{
- struct device *dev = >dev;
- int ret;
-
- gbl->def_value = device_property_read_bool(dev, "default-on");
-
- gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
- if (IS_ERR(gbl->gpiod)) {
- ret = PTR_ERR(gbl->gpiod);
-
- if (ret != -EPROBE_DEFER) {
- dev_err(dev,
- "Error: The gpios parameter is missing or
invalid.\n");
- }
- return ret;
- }
-
- return 0;
-}
-
static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
{
struct device_node *node = gbl->dev->of_node;
@@ -107,35 +84,18 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
gbl->dev = >dev;
- if (pdev->dev.fwnode) {
- ret = gpio_backlight_probe_dt(pdev, gbl);
- if (ret)
- return ret;
- } else if (pdata) {
- /*
-* Legacy platform data GPIO retrieveal. Do not expand
-* the use of this code path, currently only used by one
-* SH board.
-*/
- unsigned long flags = GPIOF_DIR_OUT;
-
+ if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = pdata->def_value;
- flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
-
- ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
- pdata ? pdata->name : "backlight");
- if (ret < 0) {
- dev_err(>dev, "unable to request GPIO\n");
- return ret;
- }
- gbl->gpiod = gpio_to_desc(pdata->gpio);
- if (!gbl->gpiod)
- return -EINVAL;
- } else {
- dev_err(>dev,
- "failed to find platform data or device tree node.\n");
- return -ENODEV;
+
+ gbl->def_value = device_property_read_bool(>dev, "default-on");
+
+ gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ if (IS_ERR(gbl->gpiod)) {
+ ret = PTR_ERR(gbl->gpiod);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev,
+ "Error: The gpios parameter is missing or
invalid.\n");
+ return ret;
}
memset(, 0, sizeof(props));
--
2.23.0
[PATCH v4 6/7] backlight: gpio: use a helper variable for >dev
From: Bartosz Golaszewski
Instead of dereferencing pdev each time, use a helper variable for
the associated device pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 20c5311c7ed2..6247687b6330 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -71,25 +71,25 @@ static int gpio_backlight_initial_power_state(struct
gpio_backlight *gbl)
static int gpio_backlight_probe(struct platform_device *pdev)
{
- struct gpio_backlight_platform_data *pdata =
- dev_get_platdata(>dev);
+ struct device *dev = >dev;
+ struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
int ret;
- gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
+ gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = >dev;
+ gbl->dev = dev;
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(>dev, "default-on");
+ gbl->def_value = device_property_read_bool(dev, "default-on");
- gbl->gpiod = devm_gpiod_get(>dev, NULL, GPIOD_ASIS);
+ gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
if (IS_ERR(gbl->gpiod)) {
ret = PTR_ERR(gbl->gpiod);
if (ret != -EPROBE_DEFER)
@@ -101,11 +101,11 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
memset(, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = 1;
- bl = devm_backlight_device_register(>dev, dev_name(>dev),
- >dev, gbl, _backlight_ops,
+ bl = devm_backlight_device_register(dev, dev_name(dev),
+ dev, gbl, _backlight_ops,
);
if (IS_ERR(bl)) {
- dev_err(>dev, "failed to register backlight\n");
+ dev_err(dev, "failed to register backlight\n");
return PTR_ERR(bl);
}
--
2.23.0
[PATCH v4 0/7] backlight: gpio: simplify the driver
From: Bartosz Golaszewski
While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.
The first patch adds all necessary data structures to ecovec24. Patch
2/7 unifies much of the code for both pdata and non-pdata cases. Patches
3-4/7 remove unused platform data fields. Last three patches contain
additional improvements for the GPIO backlight driver while we're already
modifying it.
I don't have access to this HW but hopefully this works. Only compile
tested.
[1] https://lkml.org/lkml/2019/6/25/900
v1 -> v2:
- rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
- added additional two patches with minor improvements
v2 -> v3:
- in patch 7/7: used initializers to set values for pdata and dev local vars
v3 -> v4:
- rebased on top of v5.4-rc1
- removed changes that are no longer relevant after commit ec665b756e6f
("backlight: gpio-backlight: Correct initial power state handling")
- added patch 7/7
Bartosz Golaszewski (7):
backlight: gpio: remove unneeded include
sh: ecovec24: add additional properties to the backlight device
backlight: gpio: simplify the platform data handling
sh: ecovec24: don't set unused fields in platform data
backlight: gpio: remove unused fields from platform data
backlight: gpio: use a helper variable for >dev
backlight: gpio: pull gpio_backlight_initial_power_state() into probe
arch/sh/boards/mach-ecovec24/setup.c | 33 --
drivers/video/backlight/gpio_backlight.c | 107 +--
include/linux/platform_data/gpio_backlight.h | 3 -
3 files changed, 52 insertions(+), 91 deletions(-)
--
2.23.0
[PATCH v4 2/7] sh: ecovec24: add additional properties to the backlight device
From: Bartosz Golaszewski
Add a GPIO lookup entry and a device property for GPIO backlight to the
board file. Tie them to the platform device which is now registered using
platform_device_register_full() because of the properties. These changes
are inactive now but will be used once the gpio backlight driver is
modified.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 30 +++-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index acaa97459531..aaa8ea62636f 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -371,6 +371,19 @@ static struct platform_device lcdc_device = {
},
};
+static struct gpiod_lookup_table gpio_backlight_lookup = {
+ .dev_id = "gpio-backlight.0",
+ .table = {
+ GPIO_LOOKUP("sh7724_pfc", GPIO_PTR1, NULL, GPIO_ACTIVE_HIGH),
+ { }
+ },
+};
+
+static struct property_entry gpio_backlight_props[] = {
+ PROPERTY_ENTRY_BOOL("default-on"),
+ { }
+};
+
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
.gpio = GPIO_PTR1,
@@ -378,13 +391,15 @@ static struct gpio_backlight_platform_data
gpio_backlight_data = {
.name = "backlight",
};
-static struct platform_device gpio_backlight_device = {
+static const struct platform_device_info gpio_backlight_device_info = {
.name = "gpio-backlight",
- .dev = {
- .platform_data = _backlight_data,
- },
+ .data = _backlight_data,
+ .size_data = sizeof(gpio_backlight_data),
+ .properties = gpio_backlight_props,
};
+static struct platform_device *gpio_backlight_device;
+
/* CEU0 */
static struct ceu_platform_data ceu0_pdata = {
.num_subdevs= 2,
@@ -1006,7 +1021,6 @@ static struct platform_device *ecovec_devices[]
__initdata = {
_common_device,
_device,
_device,
- _backlight_device,
_device,
_power,
#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
@@ -1462,6 +1476,12 @@ static int __init arch_setup(void)
#endif
#endif
+ gpiod_add_lookup_table(_backlight_lookup);
+ gpio_backlight_device = platform_device_register_full(
+ _backlight_device_info);
+ if (IS_ERR(gpio_backlight_device))
+ return PTR_ERR(gpio_backlight_device);
+
return platform_add_devices(ecovec_devices,
ARRAY_SIZE(ecovec_devices));
}
--
2.23.0
[RESEND PATCH v3 6/7] backlight: gpio: remove def_value from struct gpio_backlight
From: Bartosz Golaszewski
This field is unused outside of probe(). There's no need to store it.
We can make it into a local variable.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
---
drivers/video/backlight/gpio_backlight.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 70882556f047..cd6a75bca9cc 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -21,7 +21,6 @@
struct gpio_backlight {
struct device *fbdev;
struct gpio_desc *gpiod;
- int def_value;
};
static int gpio_backlight_update_status(struct backlight_device *bl)
@@ -61,7 +60,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
struct backlight_device *bl;
struct gpio_backlight *gbl;
enum gpiod_flags flags;
- int ret;
+ int ret, def_value;
gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
@@ -70,8 +69,8 @@ static int gpio_backlight_probe(struct platform_device *pdev)
if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(>dev, "default-on");
- flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+ def_value = device_property_read_bool(>dev, "default-on");
+ flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
gbl->gpiod = devm_gpiod_get(>dev, NULL, flags);
if (IS_ERR(gbl->gpiod)) {
@@ -94,7 +93,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
return PTR_ERR(bl);
}
- bl->props.brightness = gbl->def_value;
+ bl->props.brightness = def_value;
backlight_update_status(bl);
platform_set_drvdata(pdev, bl);
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/7] backlight: gpio: simplify the driver
pon., 2 wrz 2019 o 11:31 Lee Jones napisał(a): > > On Wed, 24 Jul 2019, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski > > > > While working on my other series related to gpio-backlight[1] I noticed > > that we could simplify the driver if we made the only user of platform > > data use GPIO lookups and device properties. This series tries to do > > that. > > > > The first patch adds all necessary data structures to ecovec24. Patch > > 2/7 unifies much of the code for both pdata and non-pdata cases. Patches > > 3-4/7 remove unused platform data fields. Last three patches contain > > additional improvements for the GPIO backlight driver while we're already > > modifying it. > > > > I don't have access to this HW but hopefully this works. Only compile > > tested. > > > > [1] https://lkml.org/lkml/2019/6/25/900 > > > > v1 -> v2: > > - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy > > - added additional two patches with minor improvements > > > > v2 -> v3: > > - in patch 7/7: used initializers to set values for pdata and dev local vars > > > > Bartosz Golaszewski (7): > > sh: ecovec24: add additional properties to the backlight device > > backlight: gpio: simplify the platform data handling > > sh: ecovec24: don't set unused fields in platform data > > backlight: gpio: remove unused fields from platform data > > backlight: gpio: remove dev from struct gpio_backlight > > backlight: gpio: remove def_value from struct gpio_backlight > > backlight: gpio: use a helper variable for >dev > > > > arch/sh/boards/mach-ecovec24/setup.c | 33 ++-- > > drivers/video/backlight/gpio_backlight.c | 82 +--- > > include/linux/platform_data/gpio_backlight.h | 3 - > > 3 files changed, 44 insertions(+), 74 deletions(-) > > Can you collect all your Acks and re-submit please? > Done. Bart > -- > Lee Jones [李琼斯] > Linaro Services Technical Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog
[RESEND PATCH v3 1/7] sh: ecovec24: add additional properties to the backlight device
From: Bartosz Golaszewski
Add a GPIO lookup entry and a device property for GPIO backlight to the
board file. Tie them to the platform device which is now registered using
platform_device_register_full() because of the properties. These changes
are inactive now but will be used once the gpio backlight driver is
modified.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 30 +++-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index f402aa741bf3..6926bb3865b9 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -371,6 +371,19 @@ static struct platform_device lcdc_device = {
},
};
+static struct gpiod_lookup_table gpio_backlight_lookup = {
+ .dev_id = "gpio-backlight.0",
+ .table = {
+ GPIO_LOOKUP("sh7724_pfc", GPIO_PTR1, NULL, GPIO_ACTIVE_HIGH),
+ { }
+ },
+};
+
+static struct property_entry gpio_backlight_props[] = {
+ PROPERTY_ENTRY_BOOL("default-on"),
+ { }
+};
+
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
.gpio = GPIO_PTR1,
@@ -378,13 +391,15 @@ static struct gpio_backlight_platform_data
gpio_backlight_data = {
.name = "backlight",
};
-static struct platform_device gpio_backlight_device = {
+static const struct platform_device_info gpio_backlight_device_info = {
.name = "gpio-backlight",
- .dev = {
- .platform_data = _backlight_data,
- },
+ .data = _backlight_data,
+ .size_data = sizeof(gpio_backlight_data),
+ .properties = gpio_backlight_props,
};
+static struct platform_device *gpio_backlight_device;
+
/* CEU0 */
static struct ceu_platform_data ceu0_pdata = {
.num_subdevs= 2,
@@ -1006,7 +1021,6 @@ static struct platform_device *ecovec_devices[]
__initdata = {
_common_device,
_device,
_device,
- _backlight_device,
_device,
_power,
#if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
@@ -1464,6 +1478,12 @@ static int __init arch_setup(void)
#endif
#endif
+ gpiod_add_lookup_table(_backlight_lookup);
+ gpio_backlight_device = platform_device_register_full(
+ _backlight_device_info);
+ if (IS_ERR(gpio_backlight_device))
+ return PTR_ERR(gpio_backlight_device);
+
return platform_add_devices(ecovec_devices,
ARRAY_SIZE(ecovec_devices));
}
--
2.21.0
[RESEND PATCH v3 4/7] backlight: gpio: remove unused fields from platform data
From: Bartosz Golaszewski
Remove the platform data fields that nobody uses.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
---
include/linux/platform_data/gpio_backlight.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/include/linux/platform_data/gpio_backlight.h
b/include/linux/platform_data/gpio_backlight.h
index 34179d600360..1a8b5b1946fe 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h
@@ -9,9 +9,6 @@ struct device;
struct gpio_backlight_platform_data {
struct device *fbdev;
- int gpio;
- int def_value;
- const char *name;
};
#endif
--
2.21.0
[RESEND PATCH v3 0/7] backlight: gpio: simplify the driver
From: Bartosz Golaszewski While working on my other series related to gpio-backlight[1] I noticed that we could simplify the driver if we made the only user of platform data use GPIO lookups and device properties. This series tries to do that. The first patch adds all necessary data structures to ecovec24. Patch 2/7 unifies much of the code for both pdata and non-pdata cases. Patches 3-4/7 remove unused platform data fields. Last three patches contain additional improvements for the GPIO backlight driver while we're already modifying it. I don't have access to this HW but hopefully this works. Only compile tested. [1] https://lkml.org/lkml/2019/6/25/900 v1 -> v2: - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy - added additional two patches with minor improvements v2 -> v3: - in patch 7/7: used initializers to set values for pdata and dev local vars Bartosz Golaszewski (7): sh: ecovec24: add additional properties to the backlight device backlight: gpio: simplify the platform data handling sh: ecovec24: don't set unused fields in platform data backlight: gpio: remove unused fields from platform data backlight: gpio: remove dev from struct gpio_backlight backlight: gpio: remove def_value from struct gpio_backlight backlight: gpio: use a helper variable for >dev arch/sh/boards/mach-ecovec24/setup.c | 33 ++-- drivers/video/backlight/gpio_backlight.c | 82 +--- include/linux/platform_data/gpio_backlight.h | 3 - 3 files changed, 44 insertions(+), 74 deletions(-) -- 2.21.0
[RESEND PATCH v3 3/7] sh: ecovec24: don't set unused fields in platform data
From: Bartosz Golaszewski
Platform data fields other than fbdev are no longer used by the
backlight driver. Remove them.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
---
arch/sh/boards/mach-ecovec24/setup.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/sh/boards/mach-ecovec24/setup.c
b/arch/sh/boards/mach-ecovec24/setup.c
index 6926bb3865b9..64a5a1662b6d 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -386,9 +386,6 @@ static struct property_entry gpio_backlight_props[] = {
static struct gpio_backlight_platform_data gpio_backlight_data = {
.fbdev = _device.dev,
- .gpio = GPIO_PTR1,
- .def_value = 1,
- .name = "backlight",
};
static const struct platform_device_info gpio_backlight_device_info = {
--
2.21.0
[RESEND PATCH v3 7/7] backlight: gpio: use a helper variable for >dev
From: Bartosz Golaszewski
Instead of dereferencing pdev each time, use a helper variable for
the associated device pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index cd6a75bca9cc..7dc4f90d926b 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -54,29 +54,29 @@ static const struct backlight_ops gpio_backlight_ops = {
static int gpio_backlight_probe(struct platform_device *pdev)
{
- struct gpio_backlight_platform_data *pdata =
- dev_get_platdata(>dev);
+ struct device *dev = >dev;
+ struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
enum gpiod_flags flags;
int ret, def_value;
- gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
+ gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
if (gbl == NULL)
return -ENOMEM;
if (pdata)
gbl->fbdev = pdata->fbdev;
- def_value = device_property_read_bool(>dev, "default-on");
+ def_value = device_property_read_bool(dev, "default-on");
flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
- gbl->gpiod = devm_gpiod_get(>dev, NULL, flags);
+ gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
if (IS_ERR(gbl->gpiod)) {
ret = PTR_ERR(gbl->gpiod);
if (ret != -EPROBE_DEFER) {
- dev_err(>dev,
+ dev_err(dev,
"Error: The gpios parameter is missing or
invalid.\n");
}
return ret;
@@ -85,11 +85,10 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
memset(, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = 1;
- bl = devm_backlight_device_register(>dev, dev_name(>dev),
- >dev, gbl, _backlight_ops,
- );
+ bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
+ _backlight_ops, );
if (IS_ERR(bl)) {
- dev_err(>dev, "failed to register backlight\n");
+ dev_err(dev, "failed to register backlight\n");
return PTR_ERR(bl);
}
--
2.21.0
[RESEND PATCH v3 2/7] backlight: gpio: simplify the platform data handling
From: Bartosz Golaszewski
Now that the last user of platform data (sh ecovec24) defines a proper
GPIO lookup and sets the 'default-on' device property, we can drop the
platform_data-specific GPIO handling and unify a big chunk of code.
The only field used from the platform data is now the fbdev pointer.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
Reviewed-by: Andy Shevchenko
---
drivers/video/backlight/gpio_backlight.c | 64 +---
1 file changed, 13 insertions(+), 51 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index e84f3087e29f..01262186fa1e 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -55,30 +55,6 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb = gpio_backlight_check_fb,
};
-static int gpio_backlight_probe_dt(struct platform_device *pdev,
- struct gpio_backlight *gbl)
-{
- struct device *dev = >dev;
- enum gpiod_flags flags;
- int ret;
-
- gbl->def_value = device_property_read_bool(dev, "default-on");
- flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
-
- gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
- if (IS_ERR(gbl->gpiod)) {
- ret = PTR_ERR(gbl->gpiod);
-
- if (ret != -EPROBE_DEFER) {
- dev_err(dev,
- "Error: The gpios parameter is missing or
invalid.\n");
- }
- return ret;
- }
-
- return 0;
-}
-
static int gpio_backlight_probe(struct platform_device *pdev)
{
struct gpio_backlight_platform_data *pdata =
@@ -86,6 +62,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
struct backlight_properties props;
struct backlight_device *bl;
struct gpio_backlight *gbl;
+ enum gpiod_flags flags;
int ret;
gbl = devm_kzalloc(>dev, sizeof(*gbl), GFP_KERNEL);
@@ -94,35 +71,20 @@ static int gpio_backlight_probe(struct platform_device
*pdev)
gbl->dev = >dev;
- if (pdev->dev.fwnode) {
- ret = gpio_backlight_probe_dt(pdev, gbl);
- if (ret)
- return ret;
- } else if (pdata) {
- /*
-* Legacy platform data GPIO retrieveal. Do not expand
-* the use of this code path, currently only used by one
-* SH board.
-*/
- unsigned long flags = GPIOF_DIR_OUT;
-
+ if (pdata)
gbl->fbdev = pdata->fbdev;
- gbl->def_value = pdata->def_value;
- flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
-
- ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
- pdata ? pdata->name : "backlight");
- if (ret < 0) {
- dev_err(>dev, "unable to request GPIO\n");
- return ret;
+
+ gbl->def_value = device_property_read_bool(>dev, "default-on");
+ flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+
+ gbl->gpiod = devm_gpiod_get(>dev, NULL, flags);
+ if (IS_ERR(gbl->gpiod)) {
+ ret = PTR_ERR(gbl->gpiod);
+ if (ret != -EPROBE_DEFER) {
+ dev_err(>dev,
+ "Error: The gpios parameter is missing or
invalid.\n");
}
- gbl->gpiod = gpio_to_desc(pdata->gpio);
- if (!gbl->gpiod)
- return -EINVAL;
- } else {
- dev_err(>dev,
- "failed to find platform data or device tree node.\n");
- return -ENODEV;
+ return ret;
}
memset(, 0, sizeof(props));
--
2.21.0
[RESEND PATCH v3 5/7] backlight: gpio: remove dev from struct gpio_backlight
From: Bartosz Golaszewski
This field is unused. Remove it.
Signed-off-by: Bartosz Golaszewski
Reviewed-by: Andy Shevchenko
Reviewed-by: Linus Walleij
Reviewed-by: Daniel Thompson
---
drivers/video/backlight/gpio_backlight.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c
b/drivers/video/backlight/gpio_backlight.c
index 01262186fa1e..70882556f047 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -19,9 +19,7 @@
#include
struct gpio_backlight {
- struct device *dev;
struct device *fbdev;
-
struct gpio_desc *gpiod;
int def_value;
};
@@ -69,8 +67,6 @@ static int gpio_backlight_probe(struct platform_device *pdev)
if (gbl == NULL)
return -ENOMEM;
- gbl->dev = >dev;
-
if (pdata)
gbl->fbdev = pdata->fbdev;
--
2.21.0
Re: [PATCH v3 0/7] backlight: gpio: simplify the driver
pt., 16 sie 2019 o 17:48 Bartosz Golaszewski napisał(a): > > czw., 8 sie 2019 o 10:17 Bartosz Golaszewski napisał(a): > > > > śr., 24 lip 2019 o 10:25 Bartosz Golaszewski napisał(a): > > > > > > From: Bartosz Golaszewski > > > > > > While working on my other series related to gpio-backlight[1] I noticed > > > that we could simplify the driver if we made the only user of platform > > > data use GPIO lookups and device properties. This series tries to do > > > that. > > > > > > The first patch adds all necessary data structures to ecovec24. Patch > > > 2/7 unifies much of the code for both pdata and non-pdata cases. Patches > > > 3-4/7 remove unused platform data fields. Last three patches contain > > > additional improvements for the GPIO backlight driver while we're already > > > modifying it. > > > > > > I don't have access to this HW but hopefully this works. Only compile > > > tested. > > > > > > [1] https://lkml.org/lkml/2019/6/25/900 > > > > > > v1 -> v2: > > > - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy > > > - added additional two patches with minor improvements > > > > > > v2 -> v3: > > > - in patch 7/7: used initializers to set values for pdata and dev local > > > vars > > > > > > Bartosz Golaszewski (7): > > > sh: ecovec24: add additional properties to the backlight device > > > backlight: gpio: simplify the platform data handling > > > sh: ecovec24: don't set unused fields in platform data > > > backlight: gpio: remove unused fields from platform data > > > backlight: gpio: remove dev from struct gpio_backlight > > > backlight: gpio: remove def_value from struct gpio_backlight > > > backlight: gpio: use a helper variable for >dev > > > > > > arch/sh/boards/mach-ecovec24/setup.c | 33 ++-- > > > drivers/video/backlight/gpio_backlight.c | 82 +--- > > > include/linux/platform_data/gpio_backlight.h | 3 - > > > 3 files changed, 44 insertions(+), 74 deletions(-) > > > > > > -- > > > 2.21.0 > > > > > > > Hi Rich, Yoshinori, > > > > can you ack the sh patches in this series? > > > > Bart > > Ping. Hi, any chance of getting this reviewed for sh? Bart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/7] backlight: gpio: simplify the driver
czw., 8 sie 2019 o 10:17 Bartosz Golaszewski napisał(a): > > śr., 24 lip 2019 o 10:25 Bartosz Golaszewski napisał(a): > > > > From: Bartosz Golaszewski > > > > While working on my other series related to gpio-backlight[1] I noticed > > that we could simplify the driver if we made the only user of platform > > data use GPIO lookups and device properties. This series tries to do > > that. > > > > The first patch adds all necessary data structures to ecovec24. Patch > > 2/7 unifies much of the code for both pdata and non-pdata cases. Patches > > 3-4/7 remove unused platform data fields. Last three patches contain > > additional improvements for the GPIO backlight driver while we're already > > modifying it. > > > > I don't have access to this HW but hopefully this works. Only compile > > tested. > > > > [1] https://lkml.org/lkml/2019/6/25/900 > > > > v1 -> v2: > > - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy > > - added additional two patches with minor improvements > > > > v2 -> v3: > > - in patch 7/7: used initializers to set values for pdata and dev local vars > > > > Bartosz Golaszewski (7): > > sh: ecovec24: add additional properties to the backlight device > > backlight: gpio: simplify the platform data handling > > sh: ecovec24: don't set unused fields in platform data > > backlight: gpio: remove unused fields from platform data > > backlight: gpio: remove dev from struct gpio_backlight > > backlight: gpio: remove def_value from struct gpio_backlight > > backlight: gpio: use a helper variable for >dev > > > > arch/sh/boards/mach-ecovec24/setup.c | 33 ++-- > > drivers/video/backlight/gpio_backlight.c | 82 +--- > > include/linux/platform_data/gpio_backlight.h | 3 - > > 3 files changed, 44 insertions(+), 74 deletions(-) > > > > -- > > 2.21.0 > > > > Hi Rich, Yoshinori, > > can you ack the sh patches in this series? > > Bart Ping.
Re: [PATCH v3 0/7] backlight: gpio: simplify the driver
śr., 24 lip 2019 o 10:25 Bartosz Golaszewski napisał(a): > > From: Bartosz Golaszewski > > While working on my other series related to gpio-backlight[1] I noticed > that we could simplify the driver if we made the only user of platform > data use GPIO lookups and device properties. This series tries to do > that. > > The first patch adds all necessary data structures to ecovec24. Patch > 2/7 unifies much of the code for both pdata and non-pdata cases. Patches > 3-4/7 remove unused platform data fields. Last three patches contain > additional improvements for the GPIO backlight driver while we're already > modifying it. > > I don't have access to this HW but hopefully this works. Only compile > tested. > > [1] https://lkml.org/lkml/2019/6/25/900 > > v1 -> v2: > - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy > - added additional two patches with minor improvements > > v2 -> v3: > - in patch 7/7: used initializers to set values for pdata and dev local vars > > Bartosz Golaszewski (7): > sh: ecovec24: add additional properties to the backlight device > backlight: gpio: simplify the platform data handling > sh: ecovec24: don't set unused fields in platform data > backlight: gpio: remove unused fields from platform data > backlight: gpio: remove dev from struct gpio_backlight > backlight: gpio: remove def_value from struct gpio_backlight > backlight: gpio: use a helper variable for >dev > > arch/sh/boards/mach-ecovec24/setup.c | 33 ++-- > drivers/video/backlight/gpio_backlight.c | 82 +--- > include/linux/platform_data/gpio_backlight.h | 3 - > 3 files changed, 44 insertions(+), 74 deletions(-) > > -- > 2.21.0 > Hi Rich, Yoshinori, can you ack the sh patches in this series? Bart
