On Thu, Sep 24, 2020 at 07:50:57AM -0500, Alex Elder wrote:
On 9/24/20 5:20 AM, Coiby Xu wrote:
This patch fix the following warnings from sparse,
You need to address Greg's comment.
But in general this looks good. I have one comment below, which you can address in v2. If you (or others) disagree with it, I'm fine with your code as-is. Either way, you can add this:
Reviewed-by: Alex Elder elder@linaro.org
Thank you fore reviewing this patch!
$ make C=2 drivers/staging/greybus/ drivers/staging/greybus/audio_module.c:222:25: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_module.c:222:25: expected restricted __le16 [usertype] data_cport drivers/staging/greybus/audio_module.c:222:25: got unsigned short [usertype] intf_cport_id drivers/staging/greybus/audio_topology.c:460:40: warning: restricted __le32 degrades to integer drivers/staging/greybus/audio_topology.c:691:41: warning: incorrect type in assignment (different base types)
. . .
diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c index 83b38ae8908c..56bf1a4f95ad 100644 --- a/drivers/staging/greybus/audio_topology.c +++ b/drivers/staging/greybus/audio_topology.c @@ -466,7 +466,7 @@ static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol *kcontrol, goto exit;
/* update ucontrol */
- if (gbvalue.value.integer_value[0] != val) {
- if (gbvalue.value.integer_value[0] != cpu_to_le32(val)) {
It's equivalent, but I have a small preference to convert the value from gbvalue into CPU byte order rather than what you have here.
Thank you for the suggestion! I'll use CPU byte order when submitting next version.
for (wi = 0; wi < wlist->num_widgets; wi++) { widget = wlist->widgets[wi]; snd_soc_dapm_mixer_update_power(widget->dapm, kcontrol,@@ -689,7 +689,7 @@ static int gbaudio_tplg_create_kcontrol(struct gbaudio_module_info *gb, return -ENOMEM; ctldata->ctl_id = ctl->id; ctldata->data_cport = le16_to_cpu(ctl->data_cport);
ctldata->access = ctl->access;
ctldata->access = le32_to_cpu(ctl->access); ctldata->vcount = ctl->count_values; ctldata->info = &ctl->info; *kctl = (struct snd_kcontrol_new)@@ -744,10 +744,10 @@ static int gbcodec_enum_dapm_ctl_get(struct snd_kcontrol *kcontrol, return ret; }
- ucontrol->value.enumerated.item[0] = gbvalue.value.enumerated_item[0];
- ucontrol->value.enumerated.item[0] = le32_to_cpu(gbvalue.value.enumerated_item[0]); if (e->shift_l != e->shift_r) ucontrol->value.enumerated.item[1] =
gbvalue.value.enumerated_item[1];
le32_to_cpu(gbvalue.value.enumerated_item[1]);return 0;
} @@ -801,10 +801,10 @@ static int gbcodec_enum_dapm_ctl_put(struct snd_kcontrol *kcontrol, mask = e->mask << e->shift_l;
if (gbvalue.value.enumerated_item[0] !=
ucontrol->value.enumerated.item[0]) {
cpu_to_le32(ucontrol->value.enumerated.item[0])) {
ucontrol->value.enumerated.item[0];
cpu_to_le32(ucontrol->value.enumerated.item[0]);}
if (e->shift_l != e->shift_r) {
@@ -813,10 +813,10 @@ static int gbcodec_enum_dapm_ctl_put(struct snd_kcontrol *kcontrol, val |= ucontrol->value.enumerated.item[1] << e->shift_r; mask |= e->mask << e->shift_r; if (gbvalue.value.enumerated_item[1] !=
ucontrol->value.enumerated.item[1]) {
cpu_to_le32(ucontrol->value.enumerated.item[1])) { change = 1; gbvalue.value.enumerated_item[1] =
ucontrol->value.enumerated.item[1];
cpu_to_le32(ucontrol->value.enumerated.item[1]);@@ -887,7 +887,7 @@ static int gbaudio_tplg_create_mixer_ctl(struct gbaudio_module_info *gb, return -ENOMEM; ctldata->ctl_id = ctl->id; ctldata->data_cport = le16_to_cpu(ctl->data_cport);
- ctldata->access = ctl->access;
- ctldata->access = le32_to_cpu(ctl->access); ctldata->vcount = ctl->count_values; ctldata->info = &ctl->info; *kctl = (struct snd_kcontrol_new)
-- Best regards, Coiby