greybus-dev

greybus-dev@lists.linaro.org

28 participants
853 discussions

[PATCH] staging: greybus: Drop assignment to struct pwm_chip::base

by Uwe Kleine-König

Since commit f9a8ee8c8bcd ("pwm: Always allocate PWM chip base ID dynamically") the value held in base isn't used any more in the PWM framework. All PMWs get assigned a dynamic ID, so the assignment is redundant and can be dropped. Signed-off-by: Uwe Kleine-König <u.kleine-koenig(a)pengutronix.de> --- drivers/staging/greybus/pwm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/greybus/pwm.c b/drivers/staging/greybus/pwm.c index ad20ec24031e..3fda172239d2 100644 --- a/drivers/staging/greybus/pwm.c +++ b/drivers/staging/greybus/pwm.c @@ -297,7 +297,6 @@ static int gb_pwm_probe(struct gbphy_device *gbphy_dev, pwm->dev = &gbphy_dev->dev; pwm->ops = &gb_pwm_ops; - pwm->base = -1; /* Allocate base dynamically */ pwm->npwm = pwmc->pwm_max + 1; ret = pwmchip_add(pwm); base-commit: 3123109284176b1532874591f7c81f3837bbdc17 -- 2.35.1

4 years, 4 months

[PATCH] greybus: audio_codec: initialize data to silence static checker warnings

by Xiaomeng Tong

These three declares are here: struct gbaudio_data_connection *data; Since all three of these functions check for if the codec->module_list is empty at the start of the function so these are not real bugs to bypass 'if (!data) {'. To avoid static checker warnings like Smatch, initialize the data with NULL. Signed-off-by: Xiaomeng Tong <xiam0nd.tong(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index b589cf6b1d03..939e05af4dcf 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -397,7 +397,7 @@ static int gbcodec_hw_params(struct snd_pcm_substream *substream, u8 sig_bits, channels; u32 format, rate; struct gbaudio_module_info *module; - struct gbaudio_data_connection *data; + struct gbaudio_data_connection *data = NULL; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); struct gbaudio_stream_params *params; @@ -498,7 +498,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, { int ret; struct gbaudio_module_info *module; - struct gbaudio_data_connection *data; + struct gbaudio_data_connection *data = NULL; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); struct gbaudio_stream_params *params; @@ -562,7 +562,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream) { int ret; - struct gbaudio_data_connection *data; + struct gbaudio_data_connection *data = NULL; struct gbaudio_module_info *module; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); -- 2.17.1

4 years, 5 months

[PATCH] greybus: audio_codec: fix three missing initializers for data

by Xiaomeng Tong

These three bugs are here: struct gbaudio_data_connection *data; If the list '&codec->module_list' is empty then the 'data' will keep unchanged. However, the 'data' is not initialized and filled with trash value. As a result, if the value is not NULL, the check 'if (!data) {' will always be false and never exit expectly. To fix these bug, just initialize 'data' with NULL. Cc: stable(a)vger.kernel.org Fixes: 6dd67645f22cf ("greybus: audio: Use single codec driver registration") Signed-off-by: Xiaomeng Tong <xiam0nd.tong(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index b589cf6b1d03..939e05af4dcf 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -397,7 +397,7 @@ static int gbcodec_hw_params(struct snd_pcm_substream *substream, u8 sig_bits, channels; u32 format, rate; struct gbaudio_module_info *module; - struct gbaudio_data_connection *data; + struct gbaudio_data_connection *data = NULL; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); struct gbaudio_stream_params *params; @@ -498,7 +498,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, { int ret; struct gbaudio_module_info *module; - struct gbaudio_data_connection *data; + struct gbaudio_data_connection *data = NULL; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); struct gbaudio_stream_params *params; @@ -562,7 +562,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream, static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream) { int ret; - struct gbaudio_data_connection *data; + struct gbaudio_data_connection *data = NULL; struct gbaudio_module_info *module; struct gb_bundle *bundle; struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev); -- 2.17.1

4 years, 5 months

[PATCH] staging: greybus: codecs: fix type confusion of 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. Fixes: 6dd67645f22c ("greybus: audio: Use single codec driver registration") Signed-off-by: Jakob Koschel <jakobkoschel(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index b589cf6b1d03..e19b91e7a72e 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -599,8 +599,8 @@ static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream) 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: f443e374ae131c168a065ea1748feac6b2e76613 -- 2.25.1

4 years, 5 months

[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

4 years, 5 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

4 years, 5 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, > }; >

4 years, 5 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

4 years, 5 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

4 years, 5 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

4 years, 5 months

← Newer
1
...
48
49
50
51
52
53
54
...
86
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev