say:
-#define RXFRAME_ALIGN 8
-#define RXFRAME_ALIGN_SZ (1 << RXFRAME_ALIGN)
+#define RXFRAME_ALIGN_SZ 256
This is literally used for aligning the RX frame. It seems like a crazy
thing to me that to aligned at 256 bytes. I would have expected 64 bit
alignment or page
gt;
> pattrib->ampdu_en = true;/* AGG EN */
> } else {
> -
> psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1) &
> 0xfff;
> +
hat the bug looks like to
the user. Basically if I can understand the commit message then it's
fine, and obviously "Reported by checkpatch.pl" is not sufficient to
understand what's going on.
regards,
dan carpenter
___
devel ma
t; - i2s_port, cportid,
> + i2s_port, cportid,
> AUDIO_APBRIDGEA_DIRECTION_TX);
This just looks weird now. Please, just leave it as-is.
regards,
dan carpenter
ement with Coccinelle or Smatch.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
obably a small slow down. I guess I can't be too happy about a slow
down... Let's just leave it as-is.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
if (ret < 0)
> + goto error_notifier_registered;
> +
> + return 0;
> +
> +error_notifier_registered:
> + v4l2_async_notifier_unregister(notifier);
> +error_notifier:
> + v4l2_async_notifier_cleanup(notifier);
> +error_media_entity:
> + media_entity_cleanup(>entity);
> +
> + return ret;
> +}
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
> +
> + ret = v4l2_fwnode_endpoint_parse(handle, endpoint);
> + fwnode_handle_put(handle);
> + if (ret)
> + goto error_media_entity;
> +
> + /* Notifier */
> +
> + v4l2_async_notifier_init(notifier);
> +
> + ret = v4l2_async_notifier_
ore allocations, then use gotos to unwind.
Ideally there would be a ql_deinit_device() function to mirror the
ql_init_device() function. The ql_init_device() is staging quality
code with leaks and bad label names. It should be re-written to free
things one step at a time instead of calling ql_relea
On Sat, Oct 10, 2020 at 04:18:11PM +0300, Dan Carpenter wrote:
> On Sat, Oct 10, 2020 at 02:07:13PM +0200, Jérôme Pouiller wrote:
> > On Friday 9 October 2020 20:51:01 CEST Kalle Valo wrote:
> > > CAUTION: This email originated from outside of the organization. Do not
> &g
> dereference 'tmp_buf'
> > >227 tmp_buf = kmemdup(pds->data, pds->size, GFP_KERNEL);
> > >228 ret = wfx_send_pds(wdev, tmp_buf, pds->size);
> > > ^^^
> > >229 kfree(tmp_buf);
>
r_code == 5 ||
> + return 0;
> +
> + if (work->word2.snoip.err_code == 5 ||
> work->word2.snoip.err_code == 7) {
This line is indented to match the old code and it no longer matches.
(Please update the whitespace).
>
tly in the code. Don't introduce
indirection if you can help it.
> + int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
> + int err;
> +
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driv
uct devlink *devlink;
> + struct qlge_devlink *ql_devlink;
> +
> + devlink = devlink_alloc(_devlink_ops, sizeof(struct qlge_devlink));
> + if (!devlink)
> + return -ENOMEM;
> + ql_devlink = devlink_priv(devlink);
>
> ndev = alloc_etherdev_mq(sizeof(struct ql_adapter),
>min(MAX_CPUS,
> @@ -4614,6 +4624,16 @@ static int qlge_probe(struct pci_dev *pdev,
> free_netdev(ndev);
> return err;
> }
> +
> + err = devlink_register(devlink, >dev);
> + if (err) {
> + goto devlink_free;
> + }
Checkpatch warning.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
' required = '(struct ieee80211_supported_band)->n_bitrates'
Some of these are unpublished checks that I haven't published because
they are too crap. The rest of the email is just long explanations.
Skip if not required.
regards,
dan carpenter
#1
drivers/staging/wfx/hif_tx.c:319 hif_join() erro
te it like this:
if (on == 0)
return power_down(sd);
ret = power_up(sd);
if (ret)
return ret;
return gc0310_init(sd);
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ment for readability.
Fixes: 46e4b9ec4fa4 ("staging: vchiq_arm: use list_for_each_entry when
accessing bulk_waiter_list")
Signed-off-by: Dan Carpenter
---
v2: rebase on latest linux-next
.../vc04_services/interface/vchiq_arm/vchiq_arm.c| 12 +++-
1 file changed, 7 insertions(
Smatch complains that "userdata" can be passed to vchiq_bulk_transfer()
without being initialized. This leads to a potential information leak
later on.
Fixes: a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")
Signed-off-by: Dan Carpenter
Acked-by: Arnd Berg
On Wed, Sep 30, 2020 at 12:22:37PM +0200, Arnd Bergmann wrote:
> On Wed, Sep 30, 2020 at 11:02 AM Dan Carpenter
> wrote:
> >
> > Smatch complains that "userdata" can be passed to vchiq_bulk_transfer()
> > without being initialized. Smatch is correct, however, i
time so this warning is worth silencing by setting "userdata" to
NULL.
Fixes: a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")
Signed-off-by: Dan Carpenter
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +-
1 file changed, 1 insertion(+), 1 d
The logic of this function was accidentally broken by a checkpatch
inspired cleanup. I've modified the code to restore the original
behavior and also make checkpatch happy.
Fixes: 98fe05e21a6e ("staging: rtl8712: Remove unnecesary else after return
statement.")
Signed-off-by: Dan
ret = -ENOMEM;
> goto irq_malloc;
It shouldn't do a goto, it should just return directly. The goto
releases an IRQ which isn't acquired until later in the function.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverpro
On Tue, Sep 29, 2020 at 07:34:39AM +0530, Souptick Joarder wrote:
> Hi Dan,
>
>
> On Mon, Sep 28, 2020 at 2:08 PM Dan Carpenter
> wrote:
> >
> > On Sun, Sep 27, 2020 at 08:38:04PM +0530, Souptick Joarder wrote:
> > > Inside alloc_user_pages() based
BUG_ON(). Checkpatch will complain about them. An Oops gives
basically the same information as a BUG_ON() without completely killing
the kernel so just dereferencing a NULL is preferable. Finally, we can
see from the callers that "hdm_ch" is never NULL.
regards,
dan carpent
try when
accessing bulk_waiter_list")
Signed-off-by: Dan Carpenter
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c| 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/v
gt; Fixes: 14a638ab96c5 ("media: atomisp: use pin_user_pages() for memory
> allocation")
> Signed-off-by: Souptick Joarder
> Cc: John Hubbard
> Cc: Ira Weiny
> Cc: Dan Carpenter
> ---
> drivers/staging/media/atomisp/pci/hmm/hmm_bo.c | 13 -
> 1 file changed,
EQ_BEACON_INTERVAL_WR);
> + return r;
Delete the "r" variable.
> +}
> +
> +static int hw_init(struct purelifi_chip *chip)
> +{
> + return purelifi_set_beacon_interval(chip, 100, 0, 0);
> +}
This is a oneline function which is only called once. Move it inline.
> +
> +int purelifi_chip_init_hw(struct purelifi_chip *chip)
> +{
> + int r;
> +
> + r = hw_init(chip);
> + if (r)
> + goto out;
Just return directly. The little bunny hop doesn't add anything.
> +
> + print_id(chip);
> +out:
> + return r;
> +}
Anyway, those are some ideas.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
e best would be to re-send those after the merge window, when
> > both patches will arrive upstream.
> >
> > Thanks,
> > Mauro
> That sounds more sensible. I've also just noticed that I introduced a bug in
> the first patch when rebasing it :-/, so let's hold off on the who
WZRD_DR_LOCK_BIT_MASK,
> + WZRD_USEC_POLL, WZRD_TIMEOUT_POLL);
Checkpatch will catch the missing space in "err= readl_".
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
don't need fallthrough labels for empty switch statements
like this?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
The ioremap() was never unmapped in the probe error handling or in the
remove function. The fix is to use the devm_ioremap() function so it
gets cleaned up automatically.
Fixes: 70f59c90c819 ("staging: spmi: add Hikey 970 SPMI controller driver")
Signed-off-by: Dan Carpenter
--
)
compiler: mips-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/gpu/drm/bridge/analogix/anx7625.c:1289 anx7625_get_edid() warn:
possible memory leak of 'edid'
#
https
On Fri, Sep 18, 2020 at 09:40:43AM +0300, Sakari Ailus wrote:
> Hi Dan,
>
> On Thu, Sep 17, 2020 at 01:49:41PM +0300, Dan Carpenter wrote:
> > On Thu, Sep 17, 2020 at 01:33:43PM +0300, Sakari Ailus wrote:
> > > > +static int connect_supported_devices(void)
> &
> + struct fwnode_handle *fwnode;
> +};
> + nodes[SWNODE_NULL_TERMINATOR] = SOFTWARE_NODE_NULL;
^^^^^^^^
Here.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
ht
On Thu, Sep 17, 2020 at 03:25:29PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 01:49:41PM +0300, Dan Carpenter wrote:
> > On Thu, Sep 17, 2020 at 01:33:43PM +0300, Sakari Ailus wrote:
>
> > > > + int i, ret;
> > >
> > > unsigned in
s loop and you want to unwind. With a signed variable you
can do:
while (--i >= 0)
cleanup([i]);
There are very few times where raising the type maximum from 2 billion
to 4 billion fixes anything.
regards,
dan carpenter
___
devel mailing list
d
es though.
Yeah. I suspect you should be using a different callback instead of
->sync_state() but I don't know what... :/
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
n_sensors = 0;
return ret;
The problem with cio2_bridge_unregister_sensors() is that it doesn't
clean up partial iterations through the loop. (Missing calls to
put_device(dev)).
Loops are complicated but the rest is simple. 1) Every allocation
function needs a matching cleanup function
On Mon, Sep 14, 2020 at 09:42:49AM -0700, Joe Perches wrote:
> On Mon, 2020-09-14 at 17:57 +0300, Dan Carpenter wrote:
> > On Sun, Sep 13, 2020 at 12:19:50PM +0530, Sohom Datta wrote:
> > > > From 4c8c8f3ff7f4d711daea4ac3bb987fcecc7ef1ed Mon Sep 17 00:00:00 2001
> > >
))
> plist = plist->next;
> - else if (SN_EQUAL(pnextattrib->seq_num, pattrib->seq_num))
> - return false;
> else
> break;
> }
Checkpatch is just wrong here. Ignore it when it's wrong.
regards,
dan carpent
e the memset.
Looking below, I don't think any of these are sensitive so just delete
all the memsets.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
where the last condition in the function
is different. I just fixed a bug caused by this on Wed.
https://www.spinics.net/lists/netdev/msg680226.html
But it doesn't cause any problems here so whatever...
Reviewed-by: Dan Carpenter
regards,
dan carpenter
m3554_gpio_uninit() function at all? It's basically the same as
lm3554_gpio_init() except for the order of function calls. Probably
we could just rename lm3554_gpio_init() to something like
lm3554_gpio_set_default() and use it in both the probe() and remove
functions()...
But
gt; + goto err_cleanup_entity;
> }
> return atomisp_register_i2c_module(>sd, NULL, LED_FLASH);
If atomisp_register_i2c_module() fails then we need to call
lm3554_gpio_uninit(client) and do other cleanup.
> -fail2:
> +
> +err_cleanup_entity:
> media_
09
910 atomisp_gmin_remove_subdev(sd);
911
912 del_timer_sync(>flash_off_delay);
913
914 ret = lm3554_gpio_uninit(client);
915 if (ret < 0)
916 goto fail;
917
918 kfree(flash);
919
920 return 0;
921 fail:
922 dev_err(>dev, "gpio request/direction_output fail");
923 return ret;
924 }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ts under the --- line
> drivers/staging/gdm724x/gdm_tty.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
m)->port.count)
I cannot imagine a real life example where adding these parentheses will
prevent a bug. One idea it to silence this by making dereference ops
like this a special case where checkpatch.pl doesn't suggest adding
parentheses.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ould just find something else to patch.
Probably just find a different bug and fix that instead... If at first
you don't succeed, there are tons of other stuff to work on and maybe
you will succeed there. ;)
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Really I think this function is pretty buggy. It shouldn't be falling
through at all... I reported it a couple days back so it's possible
that someone is working on a fix already.
regards,
dan carpenter
___
devel mailing list
de
ng as
well. Just get rid of the bogus fallthrough annotation.
> case ATOMISP_RUN_MODE_VIDEO:
> if (!asd->continuous_mode->val) {
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
> > - from_member().
> > > - container_from() or from_container()
> > >
> > > from_container() sounds fine, would trimming it a bit work? like
> > > from_cont().
> >
> > I'm fine with container_
sparse, "warning: dubious: !x | !y" is brought to notice
for this file. Logical and bitwise OR are basically the same in this
context so it doesn't cause a runtime bug. But let's change it to
logical OR to make it cleaner and silence the Sparse warning.
regards,
dan carpenter
>
On Tue, Aug 25, 2020 at 09:14:59AM +0100, Alex Dewar wrote:
> On Tue, Aug 25, 2020 at 10:37:13AM +0300, Dan Carpenter wrote:
> > On Mon, Aug 24, 2020 at 04:57:12PM +0100, Alex Dewar wrote:
> > > On Mon, Aug 24, 2020 at 04:19:17PM +0100, Alex Dewar wrote:
> > > &g
ng without fixing the breakage.
> >
> > Can someone cleverer than me tell me if this makes sense?
> >
> > - Alex
>
> PS -- I meant to put an RFC in the subject line and an extra tag:
> Reported-by: Dan Carpenter
>
I don't know which dev pointer we're supposed to
req->req.length);
2603 }
2604 }
2605
2606 if ((ep->epnum > 0) && (ep->direct == USB_DIR_OUT) &&
2607 (req->req.dma != 0))
2608
On Mon, Aug 24, 2020 at 02:27:08PM +0300, Dan Carpenter wrote:
> On Mon, Aug 24, 2020 at 02:24:57PM +0300, Dan Carpenter wrote:
> > On Fri, Aug 21, 2020 at 09:25:26AM -0700, Randy Dunlap wrote:
> > > On 8/21/20 8:28 AM, Tomer Samara wrote:
> > > > Remo
On Mon, Aug 24, 2020 at 02:24:57PM +0300, Dan Carpenter wrote:
> On Fri, Aug 21, 2020 at 09:25:26AM -0700, Randy Dunlap wrote:
> > On 8/21/20 8:28 AM, Tomer Samara wrote:
> > > Remove BUG() from ion_sytem_heap.c
> > >
> > > this fix the following checkpatch is
e.
>
> If some caller calls order_to_index(5), it will return -1, yes?
>
I was happy enough with the comment as-is given that I suggested it.
But an alternative comment could be "/* This is impossible.
We always pass valid values to this function. */
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
; ---
You should put a note here about what changed between v3 vs v4. Like so:
---
v4: Just remove the BUG_ON()s instead of adding new returns.
v3: Hand the new return paths or whatever.
Anyway, looks good.
Reviewed-by: Dan Carpenter
regards,
dan carpenter
_
ps: devname failed '%s'!\n", ifname);
goto err_unmap;
}
>
> RT_TRACE(COMP_INIT, "Driver probe completed1\n");
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
unt_offset;
> + ptr += 1 + pairwise_cipher_suite_size * *ptr;
The value of "*ptr" comes from skb->data. How do we know that it
doesn't point to something beyond the end of the skb->data buffer?
> + ptr += 1 + akm_suite_size * *ptr;
> +
if (ret) {
452 dev_err(dev,
453 "%d:Error while enabling %d:data
connection\n",
454 ret, dai->data_cport);
455 return ret;
456 }
457 }
Otherwise it's slightly confusing to mix the values.
The rest of the patch seems fine to me.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
static analysis tools which might complain
about this -1 return. But those tools are pretty crap. Never change
code just to make the tools happy.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
just have to keep on imagining more and
more impossible conditions. When we start trying to write code for
situations which we know are impossible that is an unending task.
> ion_page_pool_free_pages(pool, page);
> freed += (1 << pool->order);
> }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
> + __func__, kcontrol->id.name);
gb_pm_runtime_put_autosuspend(bundle) on this error path?
> + return ret;
> + }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
SHASH_DESC_ON_STACK(desc, tfm_michael);
That's probably what we should do here as well. Although I don't know
this code very well at all... This is probably the sort of change where
it would be good to have someone test it.
regards,
dan carpenter
___
gbvalue.value.integer_value[0] != val) {
> for (wi = 0; wi < wlist->num_widgets; wi++) {
> @@ -466,16 +473,10 @@ static int gbcodec_mixer_dapm_ctl_put(struct
> snd_kcontrol *kcontrol,
regards,
dan carpenter
___
devel mailing list
g. When I read the patch, of course, I could see what they were
doing but from the commit message it was impossible.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
d idea because then people assume you know what
you're doing.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
\
> +(struct device *kobj, struct device_attribute *attr, char *buf)
> \
> +{\
> + return sprintf(buf, "%d", usf_vdev.sysctl_##_value);\
> +}
> +
> +usf_show_node(up_l0_r, up_l0);
> +usf_show_node(down_r, down);
> +usf_show_node(non_ux_r, non_ux);
> +
> +usf_attr_rw(up_l0_r);
> +usf_attr_rw(down_r);
> +usf_attr_rw(non_ux_r);
> +
> +static struct attribute *sched_usf_attrs[] = {
> + _l0_r.attr,
> + _r.attr,
> + _ux_r.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(sched_usf);
> +
> +static int __init intera_monitor_init(void)
> +{
> + int res = -1;
Delete initializer
> + struct device *dev;
Get rid of the dev variable and use cpu_subsys.dev_root directly.
> +
> + res = fb_register_client(_lcd_nb);
> + if (res < 0) {
> + pr_err("Failed to register usf_lcd_nb!\n");
> + return res;
> + }
> +
> + /*
> + * create a sched_usf in cpu_subsys:
> + * /sys/devices/system/cpu/sched_usf/...
> + */
> + dev = cpu_subsys.dev_root;
> + res = sysfs_create_group(>kobj, _usf_group);
> + if (res) {
> + fb_unregister_client(_lcd_nb);
> + return res;
> + }
> +
> + return res;
"return 0;" is more readable than "return res;"
> +}
> +
> +module_init(intera_monitor_init);
> +
> +static void __exit intera_monitor_exit(void)
> +{
> + struct device *dev;
Get rid of the dev variable.
> +
> + dev = cpu_subsys.dev_root;
> + sysfs_remove_group(>kobj, _usf_group);
> + fb_unregister_client(_lcd_nb);
> + adjust_pred_demand_p = NULL;
I'm pretty sure this is not required. Delete this line.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
The "err" variable is not meaningful so there is no need to print it.
It's uninitialized on the first iteration through the loop.
Fixes: 510e340efe0c ("staging: greybus: audio: Add helper APIs for dynamic
audio modules")
Signed-off-by: Dan Carpenter
---
drivers/staging/gr
\n");
> + res = -ENOMEM;
> + goto out;
> + } else {
Delete the else. Pull the code in one indent level.
> + res = sysfs_create_group(usf_vdev.kobj, attr_group);
> + if (res) {
> + kobject_put(usf_vde
return -EINVAL;
>
> And here is a change of semantics...
Yeah. The change of semantics should be documented in the commit
message, but it's actually correct. I discussed this with Mauro earlier
but my bug reporting script didn't CC a mailing list and I didn't
catch it. Mauro sugge
Thanks!
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
sagree about style so it's easiest to go with
whatever the maintainer (Larry) wants. It's not worth debating one
way or the other so just redo it.
Then resend. Google for "how to send a v2 patch" to get the right
format.
regards,
dan carpenter
___
v4l2_err(client, "misensor_rmw_reg error exit, read failed\n");
> + v4l2_err(client, "%s exit, read failed\n", __func__);
Did you intend to delete the word "error"?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
on.
>
> Fix this by initializing it to zero.
>
> Signed-off-by: Dinghao Liu
This matches how rtl8723bs does it.
Fixes: 7b464c9fa5cc ("staging: r8188eu: Add files for new driver - part 4")
Reviewed-by: Dan Carpenter
There is a quirk/bug in GCC where it's initializing stuff to
uot;slave-addr", >i2c_addr)) {
> + if (of_property_read_u32(dev->of_node, "secondary-addr",
> >i2c_addr)) {
Sorry, I should have looked for v3 at the start to reply to it. This
line will break the driver.
regards,
dan carpenter
___
I2C_SL_CNFG);
> @@ -784,7 +784,7 @@ static int tegra_nvec_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, nvec);
> nvec->dev = dev;
>
> - if (of_property_read_u32(dev->of_node, "slave-addr", >i2c_addr)) {
> + if (
ta(pdev, nvec);
> nvec->dev = dev;
>
> - if (of_property_read_u32(dev->of_node, "slave-addr", >i2c_addr)) {
> + if (of_property_read_u32(dev->of_node, "secondary-addr",
> >i2c_addr)) {
This change will break the driver.
regards,
dan carpenter
___
On Wed, Jul 22, 2020 at 10:19:50PM +0500, Muhammad Usama Anjum wrote:
> Remove a coding style error. It makes code more readable.
>
> Signed-off-by: Muhammad Usama Anjum
> ---
This isn't a part of the official style guide so the original is fine.
regards,
d
If we fail to queue the buffer then it can never be dequeued. This can
lead to a forever loop in stop_streaming() when we wait for everything
to finish.
Fixes: 70ec64ccdaac ("staging: bcm2835-camera: Ensure all buffers are returned
on disable")
Signed-off-by: Dan Carpenter
---
&g
The allegro_mbox_init() function returns error pointers, it never
returns NULL.
Fixes: 94dc76560261 ("media: allegro: rework mbox handling")
Signed-off-by: Dan Carpenter
---
drivers/staging/media/allegro-dvt/allegro-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
;
> + pci_free_irq_vectors(dev);
Actually, my initial complaint was that I was just looking for to see if
the remove function was updated... It turns out the remove function
never freed the IRQs to begin with so it was buggy from square one.
Anyway, the remov
the disabler for MSI IRQ too with
> pci_free_irq_vectors.
No, this still has bugs. Grep the file for msi and update everything
and update the comments and label names as well.
Can you just slow down a bit and fix one driver correctly before trying
to fix a bunch of drive
Sorry, this is still not correct. See comments on other thread. There
are other msi related code which needs to be updated.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman
tives exist for.
That sounds reasonable. We can't Ack patches we haven't seen but
probably you can rename anything that you want to.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Wed, Jul 15, 2020 at 06:45:21PM +0800, Zhao wrote:
> At 2020-07-15 17:04:06, "Joe Perches" wrote:
> >On Wed, 2020-07-15 at 11:37 +0300, Dan Carpenter wrote:
> >> On Wed, Jul 15, 2020 at 09:57:55AM +0200, Greg KH wrote:
> >> > On Wed, Jul 15, 202
It's not in any archives anywhere.
> >
> > I saw it. It's here:
> > https://lore.kernel.org/lkml/20200617161127.32006-1-zhixu...@126.com/
>
> Ah, doh, sorry.
>
> Zhixu, please address the comments given to you on the series and resend
> it as a n
If gmin_camera_platform_data() returns NULL then we should return a
negative error instead of success.
Fixes: 90ebe55ab886 ("media: staging: atomisp: Add driver prefix to Kconfig
option and module names")
Signed-off-by: Dan Carpenter
---
drivers/staging/media/atomisp/i2c/ov5693/atom
On Mon, Jul 13, 2020 at 04:16:07PM +0300, Dan Carpenter wrote:
> On Sun, Jul 12, 2020 at 03:38:21PM +0300, Ivan Safonov wrote:
> > Remove unused members of struct xmit_buf: alloc_sz, ff_hwaddr,
> > dma_transfer_addr, bpending and last.
> >
> > Signed-off-by: Ivan Sa
is better than the original because now it looks like
ql_check_lb_frame(). It's still really weird looking though.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Mon, Jul 13, 2020 at 05:50:14PM +0530, Suraj Upadhyay wrote:
> Simplify while loops into more readable and simple for loops.
>
I don't think either is more clear that the other. Walter Harms hates
count down loops and he's not entirely wrong...
regards,
dan car
(len + 3) & ~3,
> > + msg_context,
> > + VCHIQ_BULK_MODE_CALLBACK);
>
> Close, you need one more space for these lines, right?
>
Yeah. The original has the space and looks correct.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
*padapter,
> return _FAIL;
>
> pxmitbuf->pbuf = PTR_ALIGN(pxmitbuf->pallocated_buf, XMITBUF_ALIGN_SZ);
Not related to this patch but kmalloc always returns data which is at
least ARCH_KMALLOC_MINALIGN aligned which is ne
> BTW, I though gcc was able to detect that, but I didn't get any warning
> on my side.
GCC stopped working. I don't know if it's because of the new pluggin
which initializes stack variables or what the heck is going on. There
has been a bug wi
This code reads from the array before verifying that "trig" is a valid
index. If the index is wildly out of bounds then reading from an
invalid address could lead to an Oops.
Fixes: a8c66b684efa ("staging: comedi: addi_apci_1500: rewrite the subdevice
support functions"
You need to have a subsystem prefix in the subject.
[PATCH] Staging: nvec: Replace HTTP links with HTTPS
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev
tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
staging-testing
head: 14442181d20490945f341644bb8257e334b01447
commit: b18ee53ad297264a79cf4ea53f20786b6455 [32/135] staging: bcm2835:
Break MMAL support out from camera
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
If
;
> Signed-off-by: Souptick Joarder
> Cc: John Hubbard
> Cc: Bharath Vedartham
> Cc: Dan Carpenter
> ---
> drivers/staging/kpc2000/kpc_dma/fileops.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/stag
201 - 300 of 4462 matches
Mail list logo
