greybus-dev

greybus-dev@lists.linaro.org

2 participants
549 discussions

[PATCH] staging: greybus: codecs: use dedicated list iterator variable

by Jakob Koschel

In preparation to limit the scope of the list iterator to the list traversal loop, use a dedicated pointer to point to the found element [1]. Link: https://lore.kernel.org/all/YhdfEIwI4EdtHdym@kroah.com/ Signed-off-by: Jakob Koschel <jakobkoschel(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index b589cf6b1d03..a446d6455fbd 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -497,7 +497,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { int ret; - struct gbaudio_module_info *module; + struct gbaudio_module_info *module = NULL, *iter; struct gbaudio_data_connection *data; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); @@ -511,11 +511,13 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, return -ENODEV; } - list_for_each_entry(module, &codec->module_list, list) { + list_for_each_entry(iter, &codec->module_list, list) { /* find the dai */ - data = find_data(module, dai->id); - if (data) + data = find_data(iter, dai->id); + if (data) { + module = iter; break; + } } if (!data) { dev_err(dai->dev, "DATA connection missing\n"); @@ -563,7 +565,7 @@ static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream) { int ret; struct gbaudio_data_connection *data; - struct gbaudio_module_info *module; + struct gbaudio_module_info *module = NULL, *iter; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); struct gbaudio_stream_params *params; @@ -592,11 +594,13 @@ static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream) return ret; } - list_for_each_entry(module, &codec->module_list, list) { + list_for_each_entry(iter, &codec->module_list, list) { /* find the dai */ - data = find_data(module, dai->id); - if (data) + data = find_data(iter, dai->id); + if (data) { + module = iter; break; + } } if (!data) { dev_err(dai->dev, "%s:%s DATA connection missing\n", base-commit: f443e374ae131c168a065ea1748feac6b2e76613 -- 2.25.1

2 years, 11 months

[PATCH] staging: greybus: codecs: fix type confusion with dedicated list iterator variable

by Jakob Koschel

If the list does not exit early then data == NULL and 'module' does not point to a valid list element. Using 'module' in such a case is not valid and was therefore removed. In preparation to limit the scope of the list iterator to the list traversal loop, use a dedicated pointer pointing to the found element [1]. Link: https://lore.kernel.org/all/YhdfEIwI4EdtHdym@kroah.com/ Signed-off-by: Jakob Koschel <jakobkoschel(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index b589cf6b1d03..0f50d1e51e2c 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -497,7 +497,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { int ret; - struct gbaudio_module_info *module; + struct gbaudio_module_info *module = NULL, *iter; struct gbaudio_data_connection *data; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); @@ -511,11 +511,13 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, return -ENODEV; } - list_for_each_entry(module, &codec->module_list, list) { + list_for_each_entry(iter, &codec->module_list, list) { /* find the dai */ - data = find_data(module, dai->id); - if (data) + data = find_data(iter, dai->id); + if (data) { + module = iter; break; + } } if (!data) { dev_err(dai->dev, "DATA connection missing\n"); @@ -563,7 +565,7 @@ static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream) { int ret; struct gbaudio_data_connection *data; - struct gbaudio_module_info *module; + struct gbaudio_module_info *module = NULL, *iter; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); struct gbaudio_stream_params *params; @@ -592,15 +594,17 @@ static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream) return ret; } - list_for_each_entry(module, &codec->module_list, list) { + list_for_each_entry(iter, &codec->module_list, list) { /* find the dai */ - data = find_data(module, dai->id); - if (data) + data = find_data(iter, dai->id); + if (data) { + module = iter; break; + } } if (!data) { - dev_err(dai->dev, "%s:%s DATA connection missing\n", - dai->name, module->name); + dev_err(dai->dev, "%s DATA connection missing\n", + dai->name); mutex_unlock(&codec->lock); return -ENODEV; } base-commit: 34e047aa16c0123bbae8e2f6df33e5ecc1f56601 -- 2.25.1

2 years, 11 months

Re: [PATCH v5] staging: greybus: introduce pwm_ops::apply

by Alex Elder

On 3/15/22 9:21 PM, Song Chen wrote: > Introduce newer .apply function in pwm_ops to replace legacy operations, > like enable, disable, config and set_polarity. It's not just "like" those four, it replaces *exactly* those four operations. > This guarantees atomic changes of the pwm controller configuration. > > Signed-off-by: Song Chen <chensong_2000(a)189.cn> I see that support for the "atomic" ->apply operation was added by commit 5ec803edcb703 ("pwm: Add core infrastructure to allow atomic updates "). And what you're doing here is removing the enable, disable, set_polarity, and config operations provided in this driver to use the new apply callback instead. And that in turn depends on a prior commit (and another commit or two after that) 43a276b003ed2 ("pwm: Introduce the pwm_state concept ") to wrap the current state stored in the device in a sub-structure. As I understand it, if the period, duty cycle, polarity, usage power, or enabled state of the device differs from the current state of the device, the new ->apply callback changes the device's state to match what is requested. Please see my comments below. > --- > v2: > 1, define duty_cycle and period as u64 in gb_pwm_config_operation. > 2, define duty and period as u64 in gb_pwm_config_request. > 3, disable before configuring duty and period if the eventual goal > is a disabled state. > > v3: > Regarding duty_cycle and period, I read more discussion in this thread, > min, warn or -EINVAL, seems no perfect way acceptable for everyone. > How about we limit their value to INT_MAX and throw a warning at the > same time when they are wrong? > > v4: > 1, explain why legacy operations are replaced. > 2, cap the value of period and duty to U32_MAX. > > v5: > 1, revise commit message. > --- > drivers/staging/greybus/pwm.c | 59 +++++++++++++++++++++-------------- > 1 file changed, 35 insertions(+), 24 deletions(-) > > diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c > index 891a6a672378..3add3032678b 100644 > --- a/drivers/staging/greybus/pwm.c > +++ b/drivers/staging/greybus/pwm.c > @@ -204,43 +204,54 @@ static void gb_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) > gb_pwm_deactivate_operation(pwmc, pwm->hwpwm); > } > > -static int gb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > - int duty_ns, int period_ns) > +static int gb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > { > + int err; > + bool enabled = pwm->state.enabled; > + u64 period = state->period; > + u64 duty_cycle = state->duty_cycle; The use of local variables here is inconsistent, and that can be confusing. Specifically, the "enabled" variable represents the *current* state, while the "period" and "duty_cycle" variables represent the *desired* state. To avoid confusion, if you're going to use local variables like that, they should all represent *either* the current state *or* the new state. Please update your patch to do one or the other. > struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > > - return gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_ns, period_ns); > -}; > + /* set polarity */ > + if (state->polarity != pwm->state.polarity) { > + if (enabled) { > + gb_pwm_disable_operation(pwmc, pwm->hwpwm); > + enabled = false; > + } > + err = gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, state->polarity); > + if (err) > + return err; > + } > > -static int gb_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, > - enum pwm_polarity polarity) > -{ > - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > + if (!state->enabled) { > + if (enabled) > + gb_pwm_disable_operation(pwmc, pwm->hwpwm); > + return 0; If you are disabling the device, you return without updating the period and duty cycle. But you *do* set polarity. Is that required by the PWM API? (I don't actually know.) Or can the polarity setting be simply ignored as well if the new state is disabled? Also, if the polarity changed, the device will have already been disabled above, so there's no need to do so again (and perhaps it might be a bad thing to do twice?). > + } > > - return gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, polarity); > -}; Since you're clamping the values to 32 bits here, your comment should explain why (because Greybus uses 32-bit values here, while the API supports 64 bit values). That would be a much more useful piece of information than "set period and duty cycle". > + /* set period and duty cycle*/ Include a space before "*/" in your comments. > + if (period > U32_MAX) > + period = U32_MAX; > > -static int gb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) > -{ > - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > + if (duty_cycle > period) > + duty_cycle = period; > > - return gb_pwm_enable_operation(pwmc, pwm->hwpwm); > -}; > + err = gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_cycle, period); > + if (err) > + return err; What if the new state set usage_power to true? It would be ignored here. Is it OK to silently ignore it? Even if it is, a comment about that would be good to see, so we know it's intentional. -Alex > -static void gb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) > -{ > - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > + /* enable/disable */ > + if (!enabled) > + return gb_pwm_enable_operation(pwmc, pwm->hwpwm); > > - gb_pwm_disable_operation(pwmc, pwm->hwpwm); > -}; > + return 0; > +} > > static const struct pwm_ops gb_pwm_ops = { > .request = gb_pwm_request, > .free = gb_pwm_free, > - .config = gb_pwm_config, > - .set_polarity = gb_pwm_set_polarity, > - .enable = gb_pwm_enable, > - .disable = gb_pwm_disable, > + .apply = gb_pwm_apply, > .owner = THIS_MODULE, > }; >

2 years, 11 months

[PATCH v4] staging: greybus: introduce pwm_ops::apply

by Song Chen

Introduce newer .apply function in pwm_ops to replace legacy operations,like enable, disable, config and set_polarity. This guarantees atomic changes of the pwm controller configuration and also reduces the size of the driver. Signed-off-by: Song Chen <chensong_2000(a)189.cn> --- v2: 1, define duty_cycle and period as u64 in gb_pwm_config_operation. 2, define duty and period as u64 in gb_pwm_config_request. 3, disable before configuring duty and period if the eventual goal is a disabled state. v3: Regarding duty_cycle and period, I read more discussion in this thread, min, warn or -EINVAL, seems no perfect way acceptable for everyone. How about we limit their value to INT_MAX and throw a warning at the same time when they are wrong? v4: 1, explain why legacy operations are replaced. 2, cap the value of period and duty to U32_MAX --- drivers/staging/greybus/pwm.c | 59 +++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c index 891a6a672378..3add3032678b 100644 --- a/drivers/staging/greybus/pwm.c +++ b/drivers/staging/greybus/pwm.c @@ -204,43 +204,54 @@ static void gb_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) gb_pwm_deactivate_operation(pwmc, pwm->hwpwm); } -static int gb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, - int duty_ns, int period_ns) +static int gb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) { + int err; + bool enabled = pwm->state.enabled; + u64 period = state->period; + u64 duty_cycle = state->duty_cycle; struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); - return gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_ns, period_ns); -}; + /* set polarity */ + if (state->polarity != pwm->state.polarity) { + if (enabled) { + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + enabled = false; + } + err = gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, state->polarity); + if (err) + return err; + } -static int gb_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, - enum pwm_polarity polarity) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + if (!state->enabled) { + if (enabled) + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + return 0; + } - return gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, polarity); -}; + /* set period and duty cycle*/ + if (period > U32_MAX) + period = U32_MAX; -static int gb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + if (duty_cycle > period) + duty_cycle = period; - return gb_pwm_enable_operation(pwmc, pwm->hwpwm); -}; + err = gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_cycle, period); + if (err) + return err; -static void gb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + /* enable/disable */ + if (!enabled) + return gb_pwm_enable_operation(pwmc, pwm->hwpwm); - gb_pwm_disable_operation(pwmc, pwm->hwpwm); -}; + return 0; +} static const struct pwm_ops gb_pwm_ops = { .request = gb_pwm_request, .free = gb_pwm_free, - .config = gb_pwm_config, - .set_polarity = gb_pwm_set_polarity, - .enable = gb_pwm_enable, - .disable = gb_pwm_disable, + .apply = gb_pwm_apply, .owner = THIS_MODULE, }; -- 2.25.1

2 years, 11 months

[PATCH v3] staging: greybus: introduce pwm_ops::apply

by Song Chen

Introduce apply in pwm_ops to replace legacy operations, like enable, disable, config and set_polarity. Signed-off-by: Song Chen <chensong_2000(a)189.cn> --- v2: 1, define duty_cycle and period as u64 in gb_pwm_config_operation. 2, define duty and period as u64 in gb_pwm_config_request. 3, disable before configuring duty and period if the eventual goal is a disabled state. v3: Regarding duty_cycle and period, I read more discussion in this thread, min, warn or -EINVAL, seems no perfect way acceptable for everyone. How about we limit their value to INT_MAX and throw a warning at the same time when they are wrong? --- drivers/staging/greybus/pwm.c | 66 +++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c index 891a6a672378..3ec5bc54d616 100644 --- a/drivers/staging/greybus/pwm.c +++ b/drivers/staging/greybus/pwm.c @@ -204,43 +204,57 @@ static void gb_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) gb_pwm_deactivate_operation(pwmc, pwm->hwpwm); } -static int gb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, - int duty_ns, int period_ns) +static int gb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) { + int err; + bool enabled = pwm->state.enabled; + u64 period = state->period; + u64 duty_cycle = state->duty_cycle; struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); - return gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_ns, period_ns); -}; - -static int gb_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, - enum pwm_polarity polarity) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); - - return gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, polarity); -}; + /* set polarity */ + if (state->polarity != pwm->state.polarity) { + if (enabled) { + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + enabled = false; + } + err = gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, state->polarity); + if (err) + return err; + } -static int gb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + if (!state->enabled) { + if (enabled) + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + return 0; + } - return gb_pwm_enable_operation(pwmc, pwm->hwpwm); -}; + /* set period and duty cycle*/ + if (period > INT_MAX) { + period = INT_MAX; + dev_warn(chip->dev, "period is %llu ns, out of range\n", state->period); + } + if (duty_cycle > INT_MAX) { + duty_cycle = INT_MAX; + dev_warn(chip->dev, + "duty cycle is %llu ns, out of range\n", state->duty_cycle); + } + err = gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_cycle, period); + if (err) + return err; -static void gb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + /* enable/disable */ + if (!enabled) + return gb_pwm_enable_operation(pwmc, pwm->hwpwm); - gb_pwm_disable_operation(pwmc, pwm->hwpwm); -}; + return 0; +} static const struct pwm_ops gb_pwm_ops = { .request = gb_pwm_request, .free = gb_pwm_free, - .config = gb_pwm_config, - .set_polarity = gb_pwm_set_polarity, - .enable = gb_pwm_enable, - .disable = gb_pwm_disable, + .apply = gb_pwm_apply, .owner = THIS_MODULE, }; -- 2.25.1

2 years, 11 months

[PATCH] staging: greybus: loopback: Fix Coding Style Error

by Husni Faiz

Macros with multiple statements should be enclosed in a do - while block. Signed-off-by: Husni Faiz <ahamedhusni73(a)gmail.com> --- drivers/staging/greybus/loopback.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 2471448ba42a..33666a49e0a8 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -162,10 +162,12 @@ static ssize_t name##_avg_show(struct device *dev, \ } \ static DEVICE_ATTR_RO(name##_avg) -#define gb_loopback_stats_attrs(field) \ - gb_loopback_ro_stats_attr(field, min, u); \ - gb_loopback_ro_stats_attr(field, max, u); \ - gb_loopback_ro_avg_attr(field) +#define gb_loopback_stats_attrs(field) \ + do { \ + gb_loopback_ro_stats_attr(field, min, u); \ + gb_loopback_ro_stats_attr(field, max, u); \ + gb_loopback_ro_avg_attr(field) \ + } while (0) #define gb_loopback_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ -- 2.25.1

2 years, 11 months

[PATCH v4 0/7] Provide and use generic_handle_irq_safe() where appropriate.

by Sebastian Andrzej Siewior

handler/ interrupt controller entry). It is low level code and the function expects that interrupts are disabled at entry point. This isn't the case for invocations from tasklets, workqueues or the primary interrupt handler on PREEMPT_RT. Once this gets noticed a "local_irq_disable|safe()" is added. To avoid further confusion this series adds generic_handle_irq_safe() which can be used from any context and adds a few user. v2…v4: - Correct kernel doc for generic_handle_irq_safe() as per Wolfram Sang. - Use "misc" instead of "mfd" for the hi6421-spmi-pmic driver. v2…v1: https://lore.kernel.org/all/20220131123404.175438-1-bigeasy@linutronix.de/ - Redo kernel-doc for generic_handle_irq_safe() in #1. - Use generic_handle_irq_safe() instead of generic_handle_irq() in the patch description where I accidently used the wrong one. v1: https://lore.kernel.org/all/20220127113303.3012207-1-bigeasy@linutronix.de/

2 years, 11 months

[PATCH] Kbuild: remove -std=gnu89 from compiler arguments

by Arnd Bergmann

From: Arnd Bergmann <arnd(a)arndb.de> During a patch discussion, Linus brought up the option of changing the C standard version from gnu89 to gnu99, which allows using variable declaration inside of a for() loop. While the C99, C11 and later standards introduce many other features, most of these are already available in gnu89 as GNU extensions as well. An earlier attempt to do this when gcc-5 started defaulting to -std=gnu11 failed because at the time that caused warnings about designated initializers with older compilers. Now that gcc-5.1 is the minimum compiler version used for building kernels, that is no longer a concern. Similarly, the behavior of 'inline' functions changes between gnu89 and gnu89, but this was taken care of by defining 'inline' to include __attribute__((gnu_inline)) in order to allow building with clang a while ago. One minor issue that remains is an added gcc warning for shifts of negative integers when building with -Werror, which happens with the 'make W=1' option, as well as for three drivers in the kernel that always enable -Werror, but it was only observed with the i915 driver so far. Nathan Chancellor reported an additional -Wdeclaration-after-statement warning that appears in a system header on arm, this still needs a workaround. Since the differences between gnu99, gnu11 and gnu17 are fairly minimal and mainly impact warnings at the -Wpedantic level that the kernel never enables, the easiest way is to just leave out the -std=gnu89 argument entirely, and rely on the compiler default language setting, which is gnu11 for gcc-5, and gnu1x/gnu17 for all other supported versions of gcc or clang. Link: https://lore.kernel.org/lkml/CAHk-=wiyCH7xeHcmiFJ-YgXUy2Jaj7pnkdKpcovt8fYbV… Link: https://github.com/ClangBuiltLinux/linux/issues/1603 Suggested-by: Linus Torvalds <torvalds(a)linux-foundation.org> Cc: Masahiro Yamada <masahiroy(a)kernel.org> Cc: linux-kbuild(a)vger.kernel.org Cc: llvm(a)lists.linux.dev Signed-off-by: Arnd Bergmann <arnd(a)arndb.de> --- I put the suggestion into patch form, based on what we discussed in the thread. I only gave it minimal testing, but it would be good to have it in linux-next if we want to do this in the merge window. --- Documentation/process/programming-language.rst | 4 ++-- .../translations/it_IT/process/programming-language.rst | 4 ++-- .../translations/zh_CN/process/programming-language.rst | 4 ++-- .../translations/zh_TW/process/programming-language.rst | 4 ++-- Makefile | 7 +++---- arch/arm64/kernel/vdso32/Makefile | 3 +-- drivers/gpu/drm/i915/Makefile | 1 + drivers/staging/greybus/tools/Makefile | 3 ++- fs/btrfs/Makefile | 1 + scripts/Makefile.extrawarn | 1 + 10 files changed, 17 insertions(+), 15 deletions(-) diff --git a/Documentation/process/programming-language.rst b/Documentation/process/programming-language.rst index ec474a70a02f..894f2a6eb9db 100644 --- a/Documentation/process/programming-language.rst +++ b/Documentation/process/programming-language.rst @@ -5,8 +5,8 @@ Programming Language The kernel is written in the C programming language [c-language]_. More precisely, the kernel is typically compiled with ``gcc`` [gcc]_ -under ``-std=gnu89`` [gcc-c-dialect-options]_: the GNU dialect of ISO C90 -(including some C99 features). ``clang`` [clang]_ is also supported, see +under ``-std=gnu11`` [gcc-c-dialect-options]_: the GNU dialect of ISO C11 +(including some C17 features). ``clang`` [clang]_ is also supported, see docs on :ref:`Building Linux with Clang/LLVM <kbuild_llvm>`. This dialect contains many extensions to the language [gnu-extensions]_, diff --git a/Documentation/translations/it_IT/process/programming-language.rst b/Documentation/translations/it_IT/process/programming-language.rst index 41db2598ce11..aa21097737ae 100644 --- a/Documentation/translations/it_IT/process/programming-language.rst +++ b/Documentation/translations/it_IT/process/programming-language.rst @@ -10,8 +10,8 @@ Linguaggio di programmazione Il kernel è scritto nel linguaggio di programmazione C [it-c-language]_. Più precisamente, il kernel viene compilato con ``gcc`` [it-gcc]_ usando -l'opzione ``-std=gnu89`` [it-gcc-c-dialect-options]_: il dialetto GNU -dello standard ISO C90 (con l'aggiunta di alcune funzionalità da C99). +l'opzione ``-std=gnu11`` [it-gcc-c-dialect-options]_: il dialetto GNU +dello standard ISO C11 (con l'aggiunta di alcune funzionalità da C17). Linux supporta anche ``clang`` [it-clang]_, leggete la documentazione :ref:`Building Linux with Clang/LLVM <kbuild_llvm>`. diff --git a/Documentation/translations/zh_CN/process/programming-language.rst b/Documentation/translations/zh_CN/process/programming-language.rst index 2a47a1d2ec20..58d2b3bd2d85 100644 --- a/Documentation/translations/zh_CN/process/programming-language.rst +++ b/Documentation/translations/zh_CN/process/programming-language.rst @@ -9,8 +9,8 @@ ============ 内核是用C语言 :ref:`c-language <cn_c-language>` 编写的。更准确地说，内核通常是用 :ref:`gcc <cn_gcc>` -在 ``-std=gnu89`` :ref:`gcc-c-dialect-options <cn_gcc-c-dialect-options>` 下编译的：ISO C90的 GNU 方言（ -包括一些C99特性） +在 ``-std=gnu11`` :ref:`gcc-c-dialect-options <cn_gcc-c-dialect-options>` 下编译的：ISO C11的 GNU 方言（ +包括一些C17特性）这种方言包含对语言 :ref:`gnu-extensions <cn_gnu-extensions>` 的许多扩展，当然，它们许多都在内核中使用。 diff --git a/Documentation/translations/zh_TW/process/programming-language.rst b/Documentation/translations/zh_TW/process/programming-language.rst index 54e3699eadf8..235de05f7e2c 100644 --- a/Documentation/translations/zh_TW/process/programming-language.rst +++ b/Documentation/translations/zh_TW/process/programming-language.rst @@ -12,8 +12,8 @@ ============ 內核是用C語言 :ref:`c-language <tw_c-language>` 編寫的。更準確地說，內核通常是用 :ref:`gcc <tw_gcc>` -在 ``-std=gnu89`` :ref:`gcc-c-dialect-options <tw_gcc-c-dialect-options>` 下編譯的：ISO C90的 GNU 方言（ -包括一些C99特性） +在 ``-std=gnu11`` :ref:`gcc-c-dialect-options <tw_gcc-c-dialect-options>` 下編譯的：ISO C11的 GNU 方言（ +包括一些C17特性）這種方言包含對語言 :ref:`gnu-extensions <tw_gnu-extensions>` 的許多擴展，當然，它們許多都在內核中使用。 diff --git a/Makefile b/Makefile index 289ce2be8032..3ff6ba766f02 100644 --- a/Makefile +++ b/Makefile @@ -432,7 +432,7 @@ HOSTCXX = g++ endif export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ - -O2 -fomit-frame-pointer -std=gnu89 + -O2 -fomit-frame-pointer export KBUILD_USERLDFLAGS := KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS) @@ -514,8 +514,7 @@ KBUILD_AFLAGS := -D__ASSEMBLY__ -fno-PIE KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \ -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE \ -Werror=implicit-function-declaration -Werror=implicit-int \ - -Werror=return-type -Wno-format-security \ - -std=gnu89 + -Werror=return-type -Wno-format-security KBUILD_CPPFLAGS := -D__KERNEL__ KBUILD_AFLAGS_KERNEL := KBUILD_CFLAGS_KERNEL := @@ -782,7 +781,7 @@ KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH) ifdef CONFIG_CC_IS_CLANG KBUILD_CPPFLAGS += -Qunused-arguments -# The kernel builds with '-std=gnu89' so use of GNU extensions is acceptable. +# The kernel builds with '-std=gnu11' so use of GNU extensions is acceptable. KBUILD_CFLAGS += -Wno-gnu # CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the # source of a reference will be _MergedGlobals and not on of the whitelisted names. diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 6c01b63ff56d..3250d0e25782 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -67,8 +67,7 @@ VDSO_CFLAGS += -DENABLE_COMPAT_VDSO=1 VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -fno-strict-aliasing -fno-common \ -Werror-implicit-function-declaration \ - -Wno-format-security \ - -std=gnu89 + -Wno-format-security VDSO_CFLAGS += -O2 # Some useful compiler-dependent flags from top-level Makefile VDSO_CFLAGS += $(call cc32-option,-Wdeclaration-after-statement,) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 1b62b9f65196..1618a6e0af4e 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -17,6 +17,7 @@ subdir-ccflags-y += -Wno-unused-parameter subdir-ccflags-y += -Wno-type-limits subdir-ccflags-y += -Wno-missing-field-initializers subdir-ccflags-y += -Wno-sign-compare +subdir-ccflags-y += -Wno-shift-negative-value subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable) subdir-ccflags-y += $(call cc-disable-warning, frame-address) subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror diff --git a/drivers/staging/greybus/tools/Makefile b/drivers/staging/greybus/tools/Makefile index ad0ae8053b79..a3bbd73171f2 100644 --- a/drivers/staging/greybus/tools/Makefile +++ b/drivers/staging/greybus/tools/Makefile @@ -12,7 +12,8 @@ CFLAGS += -std=gnu99 -Wall -Wextra -g \ -Wredundant-decls \ -Wcast-align \ -Wsign-compare \ - -Wno-missing-field-initializers + -Wno-missing-field-initializers \ + -Wno-shift-negative-value CC := $(CROSS_COMPILE)gcc diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile index 4188ba3fd8c3..99f9995670ea 100644 --- a/fs/btrfs/Makefile +++ b/fs/btrfs/Makefile @@ -17,6 +17,7 @@ subdir-ccflags-y += $(condflags) subdir-ccflags-y += -Wno-missing-field-initializers subdir-ccflags-y += -Wno-sign-compare subdir-ccflags-y += -Wno-type-limits +subdir-ccflags-y += -Wno-shift-negative-value obj-$(CONFIG_BTRFS_FS) := btrfs.o diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index 8be892887d71..650d0b8ceec3 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -36,6 +36,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) KBUILD_CFLAGS += -Wno-missing-field-initializers KBUILD_CFLAGS += -Wno-sign-compare KBUILD_CFLAGS += -Wno-type-limits +KBUILD_CFLAGS += -Wno-shift-negative-value KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1 -- 2.29.2

2 years, 12 months

[PATCH v2] staging: greybus: introduce pwm_ops::apply

by Song Chen

Introduce apply in pwm_ops to replace legacy operations, like enable, disable, config and set_polarity. Signed-off-by: Song Chen <chensong_2000(a)189.cn> --- V2: 1, define duty_cycle and period as u64 in gb_pwm_config_operation. 2, define duty and period as u64 in gb_pwm_config_request. 3, disable before configuring duty and period if the eventual goal is a disabled state. --- drivers/staging/greybus/pwm.c | 61 ++++++++++++----------- include/linux/greybus/greybus_protocols.h | 4 +- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c index 891a6a672378..03c69db5b9be 100644 --- a/drivers/staging/greybus/pwm.c +++ b/drivers/staging/greybus/pwm.c @@ -89,7 +89,7 @@ static int gb_pwm_deactivate_operation(struct gb_pwm_chip *pwmc, } static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, - u8 which, u32 duty, u32 period) + u8 which, u64 duty, u64 period) { struct gb_pwm_config_request request; struct gbphy_device *gbphy_dev; @@ -99,8 +99,8 @@ static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, return -EINVAL; request.which = which; - request.duty = cpu_to_le32(duty); - request.period = cpu_to_le32(period); + request.duty = duty; + request.period = period; gbphy_dev = to_gbphy_dev(pwmc->chip.dev); ret = gbphy_runtime_get_sync(gbphy_dev); @@ -204,43 +204,46 @@ static void gb_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) gb_pwm_deactivate_operation(pwmc, pwm->hwpwm); } -static int gb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, - int duty_ns, int period_ns) +static int gb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) { + int err; + bool enabled = pwm->state.enabled; struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); - return gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_ns, period_ns); -}; - -static int gb_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, - enum pwm_polarity polarity) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); - - return gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, polarity); -}; + /* set polarity */ + if (state->polarity != pwm->state.polarity) { + if (enabled) { + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + enabled = false; + } + err = gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, state->polarity); + if (err) + return err; + } -static int gb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + if (!state->enabled) { + if (enabled) + gb_pwm_disable_operation(pwmc, pwm->hwpwm); + return 0; + } - return gb_pwm_enable_operation(pwmc, pwm->hwpwm); -}; + /* set period and duty cycle*/ + err = gb_pwm_config_operation(pwmc, pwm->hwpwm, state->duty_cycle, state->period); + if (err) + return err; -static void gb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) -{ - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); + /* enable/disable */ + if (!enabled) + return gb_pwm_enable_operation(pwmc, pwm->hwpwm); - gb_pwm_disable_operation(pwmc, pwm->hwpwm); -}; + return 0; +} static const struct pwm_ops gb_pwm_ops = { .request = gb_pwm_request, .free = gb_pwm_free, - .config = gb_pwm_config, - .set_polarity = gb_pwm_set_polarity, - .enable = gb_pwm_enable, - .disable = gb_pwm_disable, + .apply = gb_pwm_apply, .owner = THIS_MODULE, }; diff --git a/include/linux/greybus/greybus_protocols.h b/include/linux/greybus/greybus_protocols.h index aeb8f9243545..81a6f16de098 100644 --- a/include/linux/greybus/greybus_protocols.h +++ b/include/linux/greybus/greybus_protocols.h @@ -812,8 +812,8 @@ struct gb_pwm_deactivate_request { struct gb_pwm_config_request { __u8 which; - __le32 duty; - __le32 period; + __u64 duty; + __u64 period; } __packed; struct gb_pwm_polarity_request { -- 2.25.1

3 years

Re: [PATCH v2] staging: greybus: introduce pwm_ops::apply

by Greg KH

On Tue, Feb 22, 2022 at 02:19:12PM +0800, Song Chen wrote: > Hi Greg, > > 在 2022/2/22 01:06, Greg KH 写道: > > On Fri, Feb 11, 2022 at 08:02:27PM +0800, Song Chen wrote: > > > Introduce apply in pwm_ops to replace legacy operations, > > > like enable, disable, config and set_polarity. > > > > > > Signed-off-by: Song Chen <chensong_2000(a)189.cn> > > > > > > --- > > > V2: > > > 1, define duty_cycle and period as u64 in gb_pwm_config_operation. > > > 2, define duty and period as u64 in gb_pwm_config_request. > > > 3, disable before configuring duty and period if the eventual goal > > > is a disabled state. > > > --- > > > drivers/staging/greybus/pwm.c | 61 ++++++++++++----------- > > > include/linux/greybus/greybus_protocols.h | 4 +- > > > 2 files changed, 34 insertions(+), 31 deletions(-) > > > > > > diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c > > > index 891a6a672378..03c69db5b9be 100644 > > > --- a/drivers/staging/greybus/pwm.c > > > +++ b/drivers/staging/greybus/pwm.c > > > @@ -89,7 +89,7 @@ static int gb_pwm_deactivate_operation(struct gb_pwm_chip *pwmc, > > > } > > > static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, > > > - u8 which, u32 duty, u32 period) > > > + u8 which, u64 duty, u64 period) > > > { > > > struct gb_pwm_config_request request; > > > struct gbphy_device *gbphy_dev; > > > @@ -99,8 +99,8 @@ static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, > > > return -EINVAL; > > > request.which = which; > > > - request.duty = cpu_to_le32(duty); > > > - request.period = cpu_to_le32(period); > > > + request.duty = duty; > > > + request.period = period; > > > gbphy_dev = to_gbphy_dev(pwmc->chip.dev); > > > ret = gbphy_runtime_get_sync(gbphy_dev); > > > @@ -204,43 +204,46 @@ static void gb_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) > > > gb_pwm_deactivate_operation(pwmc, pwm->hwpwm); > > > } > > > -static int gb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > > > - int duty_ns, int period_ns) > > > +static int gb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > > + const struct pwm_state *state) > > > { > > > + int err; > > > + bool enabled = pwm->state.enabled; > > > struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > > > - return gb_pwm_config_operation(pwmc, pwm->hwpwm, duty_ns, period_ns); > > > -}; > > > - > > > -static int gb_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, > > > - enum pwm_polarity polarity) > > > -{ > > > - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > > > - > > > - return gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, polarity); > > > -}; > > > + /* set polarity */ > > > + if (state->polarity != pwm->state.polarity) { > > > + if (enabled) { > > > + gb_pwm_disable_operation(pwmc, pwm->hwpwm); > > > + enabled = false; > > > + } > > > + err = gb_pwm_set_polarity_operation(pwmc, pwm->hwpwm, state->polarity); > > > + if (err) > > > + return err; > > > + } > > > -static int gb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) > > > -{ > > > - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > > > + if (!state->enabled) { > > > + if (enabled) > > > + gb_pwm_disable_operation(pwmc, pwm->hwpwm); > > > + return 0; > > > + } > > > - return gb_pwm_enable_operation(pwmc, pwm->hwpwm); > > > -}; > > > + /* set period and duty cycle*/ > > > + err = gb_pwm_config_operation(pwmc, pwm->hwpwm, state->duty_cycle, state->period); > > > + if (err) > > > + return err; > > > -static void gb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) > > > -{ > > > - struct gb_pwm_chip *pwmc = pwm_chip_to_gb_pwm_chip(chip); > > > + /* enable/disable */ > > > + if (!enabled) > > > + return gb_pwm_enable_operation(pwmc, pwm->hwpwm); > > > - gb_pwm_disable_operation(pwmc, pwm->hwpwm); > > > -}; > > > + return 0; > > > +} > > > static const struct pwm_ops gb_pwm_ops = { > > > .request = gb_pwm_request, > > > .free = gb_pwm_free, > > > - .config = gb_pwm_config, > > > - .set_polarity = gb_pwm_set_polarity, > > > - .enable = gb_pwm_enable, > > > - .disable = gb_pwm_disable, > > > + .apply = gb_pwm_apply, > > > .owner = THIS_MODULE, > > > }; > > > diff --git a/include/linux/greybus/greybus_protocols.h b/include/linux/greybus/greybus_protocols.h > > > index aeb8f9243545..81a6f16de098 100644 > > > --- a/include/linux/greybus/greybus_protocols.h > > > +++ b/include/linux/greybus/greybus_protocols.h > > > @@ -812,8 +812,8 @@ struct gb_pwm_deactivate_request { > > > struct gb_pwm_config_request { > > > __u8 which; > > > - __le32 duty; > > > - __le32 period; > > > + __u64 duty; > > > + __u64 period; > > > } __packed; > > > > Did you just change a greybus protocol message that is sent over the > > wire? Why? And why drop the endian marking of it? > > I discussed with Uwe about losing bit and found there is no perfect way to > avoid, even in Uwe's patch[1], as a result, we decided to widen duty and > period in gb_pwm_config_request, the other side of the wire is supposed to > change accordingly to support 64bit duty and period too(this will introduce > compatibility problem and there is no variable like version to address it), > similar with ktime_t in y2038, below is the detail [2] Where are you changing the "other side of the wire" here? That wire is in firmware, how are you going to coordinate that change? This is a hardware API, you can not change it without a lot of very careful work, and I still do not understand why it is needed. What "problem" is this solving that requires the protocol to change? thanks, greg k-h

3 years

← Newer
1
...
18
19
20
21
22
23
24
...
55
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev