| Message ID | 20230106091543.2440-15-kiseok.jo@irondevice.com |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: Add a driver the Iron Device SMA1303 AMP | expand |
On Fri, Jan 06, 2023 at 09:55:43AM +0000, Ki-Seok Jo wrote: > > Hi Dan, > > I'm sorry. There was an opinion that the pach sent last time was inconvenient to look at because the entire patch aws modified at once. > What you should have done was just fold everything into two patches: patch 1: add the driver patch 2: add the device tree bindings Instead you did: patch 1: add the driver patch 2: add the device tree bindings patch 3: re-write all of patch 1. Re-writing everything is not allowed, but it's also not necessary. And also it is against the rules to submit broken code and fix it later. It's a new driver so just fix patch 1 and resend that as a v2 patch. Same for the stuff I mentioned in my bug report. https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/ regards, dan carpenter
>On Fri, Jan 06, 2023 at 09:55:43AM +0000, Ki-Seok Jo wrote: > > > > Hi Dan, > > > > I'm sorry. There was an opinion that the pach sent last time was inconvenient to look at because the entire patch aws modified at once. > > > What you should have done was just fold everything into two patches: > patch 1: add the driver > patch 2: add the device tree bindings > Instead you did: > patch 1: add the driver > patch 2: add the device tree bindings > patch 3: re-write all of patch 1. > Re-writing everything is not allowed, but it's also not necessary. And also it is against the rules to submit broken code and fix it later. > It's a new driver so just fix patch 1 and resend that as a v2 patch. > Same for the stuff I mentioned in my bug report. > https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/ > regards, > dan carpenter Thank you for your kindly advise. I read your report and it was very helpful. As I understand, I already sent it the wrong way. So I want to pick up the pieces. First, I already sent the new driver code a few months ago. After that, I got several feedbacks. I've edit and test it. So a lot of things changed at once. Since I changed so many things, I didn't know what to do, so I just updated it as a patch. It's my mistake... So I already sent about patch 1 and 2, if I get the feedback, should I send a lot of changes as v2 patch?(not patch 3) For each change, should I send patch log per commit? Like that: Patch 1: add the driver Patch 2: add the device tree bindings (instead patch 3) + Patch v2 1: change 1 about feedback1 + Patch v2 2: change 2 about feedback1 + ... + Patch v2 10: change 3 about feedback1 Is it right? Or should I revise it again and send it again from v2 patch 1? (It's not registered with the kernel source yet..) Patch v2 1: add the driver (applied the feedback) Patch v2 2: add the device tree bindings I'm writing this email first because I am worried that I might send it wrong again... Thank you for your kindly help. Best regards, Kiseok Jo
On Mon, Jan 09, 2023 at 07:33:19AM +0000, Ki-Seok Jo wrote: > > >On Fri, Jan 06, 2023 at 09:55:43AM +0000, Ki-Seok Jo wrote: > > > > > > Hi Dan, > > > > > > I'm sorry. There was an opinion that the pach sent last time was inconvenient to look at because the entire patch aws modified at once. > > > > > > What you should have done was just fold everything into two patches: > > patch 1: add the driver > > patch 2: add the device tree bindings > > > Instead you did: > > patch 1: add the driver > > patch 2: add the device tree bindings > > patch 3: re-write all of patch 1. > > > Re-writing everything is not allowed, but it's also not necessary. And also it is against the rules to submit broken code and fix it later. > > > It's a new driver so just fix patch 1 and resend that as a v2 patch. > > Same for the stuff I mentioned in my bug report. > > > https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/ > > > regards, > > dan carpenter > > > Thank you for your kindly advise. I read your report and it was very helpful. > As I understand, I already sent it the wrong way. So I want to pick up the pieces. > > First, I already sent the new driver code a few months ago. > After that, I got several feedbacks. > I've edit and test it. So a lot of things changed at once. > > Since I changed so many things, I didn't know what to do, so I just updated it as a patch. > It's my mistake... > > So I already sent about patch 1 and 2, if I get the feedback, should I send a lot of changes as v2 patch?(not patch 3) > For each change, should I send patch log per commit? > > Like that: > Patch 1: add the driver > Patch 2: add the device tree bindings > > (instead patch 3) > + Patch v2 1: change 1 about feedback1 > + Patch v2 2: change 2 about feedback1 > + ... > + Patch v2 10: change 3 about feedback1 > > Is it right? No. > > Or should I revise it again and send it again from v2 patch 1? > (It's not registered with the kernel source yet..) > Patch v2 1: add the driver (applied the feedback) > Patch v2 2: add the device tree bindings > Yes. Revise again and resend everything as two patches. regards, dan carpenter
> On Mon, Jan 09, 2023 at 07:33:19AM +0000, Ki-Seok Jo wrote: > > > > >On Fri, Jan 06, 2023 at 09:55:43AM +0000, Ki-Seok Jo wrote: > > > > > > > > Hi Dan, > > > > > > > > I'm sorry. There was an opinion that the pach sent last time was inconvenient to look at because the entire patch aws modified at once. > > > > > > > > > What you should have done was just fold everything into two patches: > > > patch 1: add the driver > > > patch 2: add the device tree bindings > > > > > Instead you did: > > > patch 1: add the driver > > > patch 2: add the device tree bindings patch 3: re-write all of patch > > > 1. > > > > > Re-writing everything is not allowed, but it's also not necessary. And also it is against the rules to submit broken code and fix it later. > > > > > It's a new driver so just fix patch 1 and resend that as a v2 patch. > > > Same for the stuff I mentioned in my bug report. > > > > > https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-pat > > > ch/ > > > > > regards, > > > dan carpenter > > > > > > Thank you for your kindly advise. I read your report and it was very helpful. > > As I understand, I already sent it the wrong way. So I want to pick up the pieces. > > > > First, I already sent the new driver code a few months ago. > > After that, I got several feedbacks. > > I've edit and test it. So a lot of things changed at once. > > > > Since I changed so many things, I didn't know what to do, so I just updated it as a patch. > > It's my mistake... > > > > So I already sent about patch 1 and 2, if I get the feedback, should I > > send a lot of changes as v2 patch?(not patch 3) For each change, should I send patch log per commit? > > > > Like that: > > Patch 1: add the driver > > Patch 2: add the device tree bindings > > > > (instead patch 3) > > + Patch v2 1: change 1 about feedback1 Patch v2 2: change 2 about > > + feedback1 ... > > + Patch v2 10: change 3 about feedback1 > > > > Is it right? > No. > > > > Or should I revise it again and send it again from v2 patch 1? > > (It's not registered with the kernel source yet..) Patch v2 1: add the > > driver (applied the feedback) Patch v2 2: add the device tree bindings > > > Yes. Revise again and resend everything as two patches. > regards, > dan carpenter Thank you! I'll try again. I only updates version like v2 patch as two patches(add driver & add device tree bindings) for registering a new driver. Best regards, Kiseok Jo
diff --git a/sound/soc/codecs/sma1303.c b/sound/soc/codecs/sma1303.c index 1a5d992bf3db..4f9dab5d1613 100644 --- a/sound/soc/codecs/sma1303.c +++ b/sound/soc/codecs/sma1303.c @@ -247,7 +247,7 @@ EXPORT_SYMBOL(sma1303_set_callback_func); static int sma1303_regmap_write(struct sma1303_priv *sma1303, unsigned int reg, unsigned int val) { - int ret; + int ret = 0; int cnt = sma1303->retry_cnt; while (cnt--) { @@ -266,7 +266,7 @@ static int sma1303_regmap_write(struct sma1303_priv *sma1303, static int sma1303_regmap_update_bits(struct sma1303_priv *sma1303, unsigned int reg, unsigned int mask, unsigned int val) { - int ret; + int ret = 0; int cnt = sma1303->retry_cnt; while (cnt--) { @@ -285,7 +285,7 @@ static int sma1303_regmap_update_bits(struct sma1303_priv *sma1303, static int sma1303_regmap_read(struct sma1303_priv *sma1303, unsigned int reg, unsigned int *val) { - int ret; + int ret = 0; int cnt = sma1303->retry_cnt; while (cnt--) { @@ -772,12 +772,13 @@ static int sma1303_add_component_controls(struct snd_soc_component *component) sma1303_controls = devm_kzalloc(sma1303->dev, sizeof(sma1303_snd_controls), GFP_KERNEL); name = devm_kzalloc(sma1303->dev, - ARRAY_SIZE(sma1303_snd_controls), GFP_KERNEL); + ARRAY_SIZE(sma1303_snd_controls)*sizeof(char *), + GFP_KERNEL); for (index = 0; index < ARRAY_SIZE(sma1303_snd_controls); index++) { sma1303_controls[index] = sma1303_snd_controls[index]; name[index] = devm_kzalloc(sma1303->dev, - MAX_CONTROL_NAME, GFP_KERNEL); + MAX_CONTROL_NAME*sizeof(char), GFP_KERNEL); size = strlen(sma1303_snd_controls[index].name) + strlen(sma1303->dev->driver->name); if (!name[index] || size > MAX_CONTROL_NAME) { @@ -1544,7 +1545,7 @@ static int sma1303_i2c_probe(struct i2c_client *client, struct sma1303_priv *sma1303; struct device_node *np = client->dev.of_node; int ret, i = 0; - u32 value; + u32 value = 0; unsigned int device_info, status, otp_stat; sma1303 = devm_kzalloc(&client->dev, @@ -1564,13 +1565,13 @@ static int sma1303_i2c_probe(struct i2c_client *client, if (np) { if (!of_property_read_u32(np, "i2c-retry", &value)) { - if (value > 50 || value < 0) { + if (value > 50 || value <= 0) { sma1303->retry_cnt = SMA1303_I2C_RETRY_COUNT; dev_info(&client->dev, "%s : %s\n", __func__, "i2c-retry out of range (up to 50)"); } else { sma1303->retry_cnt = value; - dev_info(&client->dev, "%s : %s = %d\n", + dev_info(&client->dev, "%s : %s = %u\n", __func__, "i2c-retry count", value); } } else { @@ -1589,7 +1590,7 @@ static int sma1303_i2c_probe(struct i2c_client *client, } if (!of_property_read_u32(np, "tdm-slot-tx", &value)) { dev_info(&client->dev, - "tdm slot tx is '%d' from DT\n", value); + "tdm slot tx is '%u' from DT\n", value); sma1303->tdm_slot_tx = value; } else { dev_info(&client->dev, @@ -1609,7 +1610,7 @@ static int sma1303_i2c_probe(struct i2c_client *client, break; default: dev_err(&client->dev, - "Invalid sys-clk-id: %d\n", value); + "Invalid sys-clk-id: %u\n", value); return -EINVAL; } sma1303->sys_clk_id = value;
Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com> Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <error27@gmail.com> --- sound/soc/codecs/sma1303.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)