Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16' preferred over 'int16_t'
-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Monday, November 14, 2016 4:06 AM
To: Stuart Yoder
Cc: Shiva Kerdel ; de...@driverdev.osuosl.org;
gre...@linuxfoundation.org; linux-
ker...@vger.kernel.org; Nipun Gupta ; tred...@nvidia.com;
Laurentiu Tudor
Subject: Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16'
preferred over 'int16_t'
On Fri, Nov 11, 2016 at 02:52:31PM +, Stuart Yoder wrote:
diff --git a/drivers/staging/fsl-mc/include/mc-bus.h
b/drivers/staging/fsl-mc/include/mc-bus.h
index e915574..c7cad87 100644
--- a/drivers/staging/fsl-mc/include/mc-bus.h
+++ b/drivers/staging/fsl-mc/include/mc-bus.h
@@ -42,8 +42,8 @@ struct msi_domain_info;
*/
struct fsl_mc_resource_pool {
enum fsl_mc_pool_type type;
- int16_t max_count;
- int16_t free_count;
+ s16 max_count;
My understanding is that this has to be signed because the design of
this driver is that we keep adding devices until the the counter
overflows. After that there are a couple tests for
"if (WARN_ON(res_pool->max_count < 0)) " which prevent the driver from
working again.
This all seems pretty horrible.
Can you elaborate?
The resource pools managed by this driver are populated by hardware objects
discovered when the fsl-mc bus probes a DPRC/container.
The number of potential objects discovered of a given type is in the hundreds,
so a signed 16-bit number is order of magnitudes larger than anything we will
ever encounter.
Would you feel better about this if max_count was an int?
Yeah.
The max_count reflects the total number of objects discovered. If that is
exceeded we display a warning, because something is horribly wrong. Nothing
stops working, the allocator simply refuses to add anything else to the
free list.
I didn't look at this carefully... Anyway we can't remove devices
either. If we just had an upper bound instead of overflowing the s16
then we could still remove devices.
The only reason max_count is there at all is as an internal check against
bugs and resource leaks. If the driver is being removed and a resource
pool is being freed, max_count must be zero...i.e. all objects should have
been removed. If not, there is a leak somewhere. So, it's a sanity check.
Just use a normal upper bound with a #define instead of an magic number
hidden and then disguised as an integer overflow.
Ok, agree that it would be clearer like that.
Shiva, can you respin this patch and just make both max_count and free_count
to be of type "int".
I will get Dan's suggestion sent as a separate patch...to #define the upper
bound
instead of relying on integer overflow.
Thanks,
Stuart
I will do that, thank you for the clarification of what I should do.
Thanks,
Shiva Kerdel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Patch procedure
On Mon, Nov 14, 2016 at 12:16:08PM -0500, feas wrote: > Here is how I am going about making the patches. It is basically > what I have picked up from kernel newbies among other sites > and videos on making patches. I would be greatful for any > pointers on what seems to be the problem(s) with why it does > not produce a proper patch. Honestly, it's not our job to review someone's patch creation proceedures and notes as everyone does it differently. We will be glad to review your patches that you create. I think the review that I previously provided should be sufficient to start with. Again, send a short patch series to verify this, do not send 100+ patches without getting feedback on them. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Problems with item delivery, n.2553686726
Hello, We could not deliver your parcel. You can review complete details of your order in the find attached. Steffanie Iverson - Area Manager FedEx , CA Regards FedEx.doc Description: MS-Word document ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 3/3] PCI: hv: delete the device earlier from hbus->children for hot-remove
> -Original Message- > From: Dexuan Cui > Sent: Wednesday, November 9, 2016 11:20 PM > To: Bjorn Helgaas ; linux-...@vger.kernel.org; > de...@linuxdriverproject.org > Cc: gre...@linuxfoundation.org; KY Srinivasan ; > Haiyang Zhang ; Stephen Hemminger > ; Jake Oshins ; Hadden > Hoppert ; Vitaly Kuznetsov > ; jasow...@redhat.com; a...@canonical.com; > o...@aepfle.de; linux-ker...@vger.kernel.org > Subject: [PATCH 3/3] PCI: hv: delete the device earlier from hbus->children > for hot-remove > > After we send a PCI_EJECTION_COMPLETE message to the host, the host will > immediately send us a PCI_BUS_RELATIONS message with > relations->device_count == 0, so pci_devices_present_work(), running on > another thread, can find the being-ejected device, mark > the hpdev->reported_missing to true, and run list_move_tail()/list_del() > for the device -- this races hv_eject_device_work() -> list_del(). > > The patch moves the list_del() in hv_eject_device_work() to an earlier > place, i.e. before we send PCI_EJECTION_COMPLETE, so later the > pci_devices_present_work() can't see the device. > > Signed-off-by: Dexuan Cui > CC: Jake Oshins > Cc: KY Srinivasan > CC: Haiyang Zhang > CC: Vitaly Kuznetsov Thanks Dexuan. Acked-by: K. Y. Srinivasan > --- > drivers/pci/host/pci-hyperv.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 7590ad0..fe5179d 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1582,6 +1582,10 @@ static void hv_eject_device_work(struct > work_struct *work) > pci_dev_put(pdev); > } > > + spin_lock_irqsave(&hpdev->hbus->device_list_lock, flags); > + list_del(&hpdev->list_entry); > + spin_unlock_irqrestore(&hpdev->hbus->device_list_lock, flags); > + > memset(&ctxt, 0, sizeof(ctxt)); > ejct_pkt = (struct pci_eject_response *)&ctxt.pkt.message; > ejct_pkt->message_type.type = PCI_EJECTION_COMPLETE; > @@ -1590,10 +1594,6 @@ static void hv_eject_device_work(struct > work_struct *work) >sizeof(*ejct_pkt), (unsigned long)&ctxt.pkt, >VM_PKT_DATA_INBAND, 0); > > - spin_lock_irqsave(&hpdev->hbus->device_list_lock, flags); > - list_del(&hpdev->list_entry); > - spin_unlock_irqrestore(&hpdev->hbus->device_list_lock, flags); > - > put_pcichild(hpdev, hv_pcidev_ref_childlist); > put_pcichild(hpdev, hv_pcidev_ref_pnp); > put_hvpcibus(hpdev->hbus); > -- > 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/3] PCI: hv: fix hv_pci_remove() for hot-remove
> -Original Message-
> From: Dexuan Cui
> Sent: Wednesday, November 9, 2016 11:19 PM
> To: Bjorn Helgaas ; linux-...@vger.kernel.org;
> de...@linuxdriverproject.org
> Cc: gre...@linuxfoundation.org; KY Srinivasan ;
> Haiyang Zhang ; Stephen Hemminger
> ; Jake Oshins ; Hadden
> Hoppert ; Vitaly Kuznetsov
> ; jasow...@redhat.com; a...@canonical.com;
> o...@aepfle.de; linux-ker...@vger.kernel.org
> Subject: [PATCH 2/3] PCI: hv: fix hv_pci_remove() for hot-remove
>
> 1. We don't really need such a big on-stack buffer when sending
> the teardown_packet: vmbus_sendpacket() here only uses
> sizeof(struct pci_message).
>
> 2. In the hot-remove case (PCI_EJECT), after we send
> PCI_EJECTION_COMPLETE
> to the host, the host will send a RESCIND_CHANNEL message to us and the
> host won't access the per-channel ringbuffer any longer, so we needn't
> send PCI_RESOURCES_RELEASED/PCI_BUS_D0EXIT to the host, and we
> shouldn't
> expect the host's completion message of PCI_BUS_D0EXIT, which will never
> come.
>
> 3. We should send PCI_BUS_D0EXIT after hv_send_resources_released().
>
> Signed-off-by: Dexuan Cui
> CC: Jake Oshins
> Cc: KY Srinivasan
> CC: Haiyang Zhang
> CC: Vitaly Kuznetsov
Thanks Dexuan.
Acked-by: K. Y. Srinivasan
>
> ---
>
> I made the patch based on discussions with Jake Oshins and others.
>
> drivers/pci/host/pci-hyperv.c | 53 +++
>
> 1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 93ed64a..7590ad0 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -2266,24 +2266,32 @@ static int hv_pci_probe(struct hv_device *hdev,
> return ret;
> }
>
> -/**
> - * hv_pci_remove() - Remove routine for this VMBus channel
> - * @hdev:VMBus's tracking struct for this root PCI bus
> - *
> - * Return: 0 on success, -errno on failure
> - */
> -static int hv_pci_remove(struct hv_device *hdev)
> +static void hv_pci_bus_exit(struct hv_device *hdev)
> {
> - int ret;
> - struct hv_pcibus_device *hbus;
> - union {
> + struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
> + struct {
> struct pci_packet teardown_packet;
> - u8 buffer[0x100];
> + u8 buffer[sizeof(struct pci_message)];
> } pkt;
> struct pci_bus_relations relations;
> struct hv_pci_compl comp_pkt;
> + int ret;
>
> - hbus = hv_get_drvdata(hdev);
> + /*
> + * After the host sends the RESCIND_CHANNEL message, it doesn't
> + * access the per-channel ringbuffer any longer.
> + */
> + if (hdev->channel->rescind)
> + return;
> +
> + /* Delete any children which might still exist. */
> + memset(&relations, 0, sizeof(relations));
> + hv_pci_devices_present(hbus, &relations);
> +
> + ret = hv_send_resources_released(hdev);
> + if (ret)
> + dev_err(&hdev->device,
> + "Couldn't send resources released packet(s)\n");
>
> memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet));
> init_completion(&comp_pkt.host_event);
> @@ -2298,7 +2306,19 @@ static int hv_pci_remove(struct hv_device *hdev)
>
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> if (!ret)
> wait_for_completion_timeout(&comp_pkt.host_event, 10 *
> HZ);
> +}
> +
> +/**
> + * hv_pci_remove() - Remove routine for this VMBus channel
> + * @hdev:VMBus's tracking struct for this root PCI bus
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int hv_pci_remove(struct hv_device *hdev)
> +{
> + struct hv_pcibus_device *hbus;
>
> + hbus = hv_get_drvdata(hdev);
> if (hbus->state == hv_pcibus_installed) {
> /* Remove the bus from PCI's point of view. */
> pci_lock_rescan_remove();
> @@ -2307,17 +2327,10 @@ static int hv_pci_remove(struct hv_device
> *hdev)
> pci_unlock_rescan_remove();
> }
>
> - ret = hv_send_resources_released(hdev);
> - if (ret)
> - dev_err(&hdev->device,
> - "Couldn't send resources released packet(s)\n");
> + hv_pci_bus_exit(hdev);
>
> vmbus_close(hdev->channel);
>
> - /* Delete any children which might still exist. */
> - memset(&relations, 0, sizeof(relations));
> - hv_pci_devices_present(hbus, &relations);
> -
> iounmap(hbus->cfg_addr);
> hv_free_config_window(hbus);
> pci_free_resource_list(&hbus->resources_for_children);
> --
> 2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/3] PCI: hv: use the correct buffer size in new_pcichild_device()
> -Original Message-
> From: Dexuan Cui
> Sent: Wednesday, November 9, 2016 11:18 PM
> To: Bjorn Helgaas ; linux-...@vger.kernel.org;
> de...@linuxdriverproject.org
> Cc: gre...@linuxfoundation.org; KY Srinivasan ;
> Haiyang Zhang ; Stephen Hemminger
> ; Jake Oshins ; Hadden
> Hoppert ; Vitaly Kuznetsov
> ; jasow...@redhat.com; a...@canonical.com;
> o...@aepfle.de; linux-ker...@vger.kernel.org
> Subject: [PATCH 1/3] PCI: hv: use the correct buffer size in
> new_pcichild_device()
>
> We don't really need such a big on-stack buffer.
> vmbus_sendpacket() here only uses sizeof(struct pci_child_message).
>
> Signed-off-by: Dexuan Cui
> CC: Jake Oshins
> Cc: KY Srinivasan
> CC: Haiyang Zhang
> CC: Vitaly Kuznetsov
Thanks Dexuan.
Acked-by: K. Y. Srinivasan
> ---
> drivers/pci/host/pci-hyperv.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 763ff87..93ed64a 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1271,9 +1271,9 @@ static struct hv_pci_dev
> *new_pcichild_device(struct hv_pcibus_device *hbus,
> struct hv_pci_dev *hpdev;
> struct pci_child_message *res_req;
> struct q_res_req_compl comp_pkt;
> - union {
> - struct pci_packet init_packet;
> - u8 buffer[0x100];
> + struct {
> + struct pci_packet init_packet;
> + u8 buffer[sizeof(struct pci_child_message)];
> } pkt;
> unsigned long flags;
> int ret;
> --
> 2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] [STYLE]staging:vme:vme_user.c Correct spelling mistakes
On Sun, Nov 13, 2016 at 08:28:34PM -0500, Walt Feasel wrote:
> Make spelling corrections for 'correctly' and
> 'unregister'.
>
> Signed-off-by: Walt Feasel
Acked-by: Martyn Welch
> ---
>
> Removed the previously submitted U.S. spelling for
> 'initialise' to keep the correct U.K. spelling.
>
> drivers/staging/vme/devices/vme_user.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/vme/devices/vme_user.c
> b/drivers/staging/vme/devices/vme_user.c
> index 5dd430f..2753fb2 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -661,7 +661,7 @@ static int vme_user_probe(struct vme_dev *vdev)
> }
> class_destroy(vme_user_sysfs_class);
>
> - /* Ensure counter set correcty to unalloc all master windows */
> + /* Ensure counter set correctly to unalloc all master windows */
> i = MASTER_MAX + 1;
> err_master:
> while (i > MASTER_MINOR) {
> @@ -671,7 +671,7 @@ static int vme_user_probe(struct vme_dev *vdev)
> }
>
> /*
> - * Ensure counter set correcty to unalloc all slave windows and buffers
> + * Ensure counter set correctly to unalloc all slave windows and buffers
>*/
> i = SLAVE_MAX + 1;
> err_slave:
> @@ -716,7 +716,7 @@ static int vme_user_remove(struct vme_dev *dev)
> /* Unregister device driver */
> cdev_del(vme_user_cdev);
>
> - /* Unregiser the major and minor device numbers */
> + /* Unregister the major and minor device numbers */
> unregister_chrdev_region(MKDEV(VME_MAJOR, 0), VME_DEVS);
>
> return 0;
> --
> 2.1.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: ni_mio_common: fix M Series ni_ai_insn_read() data mask
For NI M Series cards, the Comedi `insn_read` handler for the AI
subdevice is broken due to ANDing the value read from the AI FIFO data
register with an incorrect mask. The incorrect mask clears all but the
most significant bit of the sample data. It should preserve all the
sample data bits. Correct it.
Fixes: 817144ae7fda ("staging: comedi: ni_mio_common: remove unnecessary use of
'board->adbits'")
Signed-off-by: Ian Abbott
Cc: # 4.7+
---
Needs backporting to stable kernels >= 3.17
---
drivers/staging/comedi/drivers/ni_mio_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 9812508..9543539 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -1832,7 +1832,7 @@ static int ni_ai_insn_read(struct comedi_device *dev,
unsigned int *data)
{
struct ni_private *devpriv = dev->private;
- unsigned int mask = (s->maxdata + 1) >> 1;
+ unsigned int mask = s->maxdata;
int i, n;
unsigned int signbits;
unsigned int d;
--
2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: ni_mio_common: fix E series ni_ai_insn_read() data
Commit 0557344e2149 ("staging: comedi: ni_mio_common: fix local var for
32-bit read") changed the type of local variable `d` from `unsigned
short` to `unsigned int` to fix a bug introduced in
commit 9c340ac934db ("staging: comedi: ni_stc.h: add read/write
callbacks to struct ni_private") when reading AI data for NI PCI-6110
and PCI-6111 cards. Unfortunately, other parts of the function rely on
the variable being `unsigned short` when an offset value in local
variable `signbits` is added to `d` before writing the value to the
`data` array:
d += signbits;
data[n] = d;
The `signbits` variable will be non-zero in bipolar mode, and is used to
convert the hardware's 2's complement, 16-bit numbers to Comedi's
straight binary sample format (with 0 representing the most negative
voltage). This breaks because `d` is now 32 bits wide instead of 16
bits wide, so after the addition of `signbits`, `data[n]` ends up being
set to values above 65536 for negative voltages. This affects all
supported "E series" cards except PCI-6143 (and PXI-6143). Fix it by
ANDing the value written to the `data[n]` with the mask 0x.
Fixes: 0557344e2149 ("staging: comedi: ni_mio_common: fix local var for 32-bit
read")
Signed-off-by: Ian Abbott
Cc: # 4.7+
Needs backporting to stable kernels 3.18 onwards.
---
drivers/staging/comedi/drivers/ni_mio_common.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 9543539..a974ab7 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -1875,7 +1875,7 @@ static int ni_ai_insn_read(struct comedi_device *dev,
return -ETIME;
}
d += signbits;
- data[n] = d;
+ data[n] = d & 0x;
}
} else if (devpriv->is_6143) {
for (n = 0; n < insn->n; n++) {
@@ -1924,9 +1924,8 @@ static int ni_ai_insn_read(struct comedi_device *dev,
data[n] = dl;
} else {
d = ni_readw(dev, NI_E_AI_FIFO_DATA_REG);
- /* subtle: needs to be short addition */
d += signbits;
- data[n] = d;
+ data[n] = d & 0x;
}
}
}
--
2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: ni_mio_common: remove variable 'dl' in ni_ai_insn_read()
In `ni_ai_insn_read()`, local variable `dl` is declared as `unsigned
long`, but `unsigned int` will do. Get rid of it and use local variable
`d` instead. (That used to be `unsigned short`, but has been `unsigned
int` since kernel version 3.18.)
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_mio_common.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index a974ab7..b2e3828 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -1836,7 +1836,6 @@ static int ni_ai_insn_read(struct comedi_device *dev,
int i, n;
unsigned int signbits;
unsigned int d;
- unsigned long dl;
ni_load_channelgain_list(dev, s, 1, &insn->chanspec);
@@ -1887,15 +1886,15 @@ static int ni_ai_insn_read(struct comedi_device *dev,
* bit to move a single 16bit stranded sample into
* the FIFO.
*/
- dl = 0;
+ d = 0;
for (i = 0; i < NI_TIMEOUT; i++) {
if (ni_readl(dev, NI6143_AI_FIFO_STATUS_REG) &
0x01) {
/* Get stranded sample into FIFO */
ni_writel(dev, 0x01,
NI6143_AI_FIFO_CTRL_REG);
- dl = ni_readl(dev,
- NI6143_AI_FIFO_DATA_REG);
+ d = ni_readl(dev,
+NI6143_AI_FIFO_DATA_REG);
break;
}
}
@@ -1903,7 +1902,7 @@ static int ni_ai_insn_read(struct comedi_device *dev,
dev_err(dev->class_dev, "timeout\n");
return -ETIME;
}
- data[n] = (((dl >> 16) & 0x) + signbits) & 0x;
+ data[n] = (((d >> 16) & 0x) + signbits) & 0x;
}
} else {
for (n = 0; n < insn->n; n++) {
@@ -1919,9 +1918,9 @@ static int ni_ai_insn_read(struct comedi_device *dev,
return -ETIME;
}
if (devpriv->is_m_series) {
- dl = ni_readl(dev, NI_M_AI_FIFO_DATA_REG);
- dl &= mask;
- data[n] = dl;
+ d = ni_readl(dev, NI_M_AI_FIFO_DATA_REG);
+ d &= mask;
+ data[n] = d;
} else {
d = ni_readw(dev, NI_E_AI_FIFO_DATA_REG);
d += signbits;
--
2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] staging: comedi: ni_mio_common: ni_ai_insn_read() fixes
A couple of bug fixes for incorrect sample data read by ni_ai_insn_read(), plus removal of an unnecessary variable. 1) staging: comedi: ni_mio_common: fix M Series ni_ai_insn_read() data mask 2) staging: comedi: ni_mio_common: fix E series ni_ai_insn_read() data 3) staging: comedi: ni_mio_common: remove variable 'dl' in ni_ai_insn_read() The bug fix patches 1) and 2) apply cleanly to kernel version 4.7 onwards, but will need backporting to longterm kernels 3.18 onwards. drivers/staging/comedi/drivers/ni_mio_common.c | 22 ++ 1 file changed, 10 insertions(+), 12 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/35] second batch of missing lustre 2.8 patches
> On Thu, Nov 10, 2016 at 12:30:30PM -0500, James Simmons wrote: > > More fixes missing from the upstream client. Also a nice cleanup > > with the removal of cl_req which is no longer needed. More cleanup > > for lustre_idl.h which is a uapi header. Like the last batch these > > patches are independent of order. > > I didn't apply a few of these (string parsing stuff, and build > breakages.) > > What's the plan for getting this out of staging? I feel like you all > are still adding new features along with these "cleanups". Normally > that's fine, but I really really want to get this out of staging as it's > been there for way too long. When is that going to happen? First I should address why the push to bring it into sync with our prouction code base. One was to make it attractive to our user base. In my other email I explained that. Second the feed back here has been so valuable. We are at the point where bugs we haven't found are being reported here and addressed. Also your input has made the Lustre developers to reflect on what we have done. In a way leaving staging will be sad since that will stop :-( Lastly I really didn't want to cleanup the lustre client and then when it left staging do this massive dump of changes without people like Julia, Dan and you looking at it. I just felt that wouldn't of been right. We are super close to reaching a very important mile stone of reaching lustre 2.8.0 level of suppport. At this point we can stop at our lustre 2.8.0 release for the sync since currently most the lustre users out their are staying at that version. Secondly the major of patches landed to our soon to be release 2.9 version was for the patches missing from the staging tree. So before we consider moving out of staging some gaps need to be filled. The zero day system has found bugs on platforms we don't have access too. We really need to hook into that system. Also Julia Lawall Coccinelle work has been really wonderful. Intel does have a mirror of your tree and have started the integration of the test harness. For my work I have using a local private test harness setup. This Intel one is planned for public consumption. What is done here with Coccinelle and zero day needs to be intergrated. So this is what needs to be done from our side. Now for what is required to leave the staging tree. Honestly I can think of many many many things that need to be done. The question becomes what has to be done before leaving staging verses what can be completed after leaving staging. We have the normal style issues and checkpatch issues which is not much anymore. Then their is the uapi header cleanup. What else beyond that? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vc04_services: rework ioctl code path
On Mon, 2016-11-14 at 12:48 +0300, Dan Carpenter wrote:
> On Thu, Nov 10, 2016 at 10:15:31PM -0800, Michael Zoran wrote:
> > +static void *
> > +vchiq_ioctl_kmalloc(struct vchiq_ioctl_call_context *ctxt, size_t
> > size)
> > +{
> > + void *mem;
> > +
> > + if (!ctxt->stackmem_used && size < sizeof(ctxt->stackmem))
> > {
> > + ctxt->stackmem_used = true;
> > + return ctxt->stackmem;
> > + }
> > +
> > + mem = kmalloc(size + sizeof(void *), GFP_KERNEL);
>
> This is a potential integer overflow leading to corruption. I don't
> understand why we need this complicated memory management anyway...
>
You could be right. This patch was very large and it hasn't received
the review that it probably should get. Also the checkpatch.pl
utility is complaining about obsolete kernel functionality that the old
code had and I really don't have the time to redo.
Perhaps the entire patch should be removed from consideration until I
can possibly work out a V3?
> > + if (!mem)
> > + return NULL;
> > +
> > + *(void **)mem = ctxt->prev_kmalloc;
> > + ctxt->prev_kmalloc = mem;
> > +
> > + return mem + sizeof(void *);
> > +}
>
> regards,
> dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: lustre: mdc: manage number of modify RPCs in flight
> On Mon, Nov 14, 2016 at 04:59:48PM +, James Simmons wrote: > > > > > On Thu, Nov 10, 2016 at 10:51:13AM -0500, James Simmons wrote: > > > > From: Gregoire Pichon > > > > > > > > This patch is the main client part of a new feature that supports > > > > multiple modify metadata RPCs in parallel. Its goal is to improve > > > > metadata operations performance of a single client, while maintening > > > > the consistency of MDT reply reconstruction and MDT recovery > > > > mechanisms. > > > > > > > > It allows to manage the number of modify RPCs in flight within > > > > the client obd structure and to assign a virtual index (the tag) to > > > > each modify RPC to help server side cleaning of reply data. > > > > > > > > The mdc component uses this feature to send multiple modify RPCs > > > > in parallel. > > > > > > Is this a new feature? Why should we take this now and not just wait > > > until the code is out of staging? > > > > Yes on the server side. So the problem on our meta data servers couldn't > > handle writing mulitiple bits of data to the back end disk at ths same > > time. > > > > One client side the issue was the metadata operations were being > > serialized by a mutex in the MDC layer. That is what this patch fixed. > > So for the client it would be a performance improvement patch. > > So, it's a "performance" patch, which isn't functionality, so why should > we merge this to staging now? Why aren't people working on the known > coding issues to get this out of staging instead of working on > performance stuff? Because the primary goal which the people at my company, not Intel by the way, wanted was to get this to what is running in production environments so people would actually use the staging client. When it was old and broken no one would touch it with a 10 foot pole. The currently supported verison in production is lustre 2.8.0 so we are only about 30 patches away from reaching the goal. I'm going to respond to your other email in length about leaving staging. It would be really nice for the user base if we can reach that goal. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: lustre: mdc: manage number of modify RPCs in flight
On Mon, Nov 14, 2016 at 04:59:48PM +, James Simmons wrote: > > > On Thu, Nov 10, 2016 at 10:51:13AM -0500, James Simmons wrote: > > > From: Gregoire Pichon > > > > > > This patch is the main client part of a new feature that supports > > > multiple modify metadata RPCs in parallel. Its goal is to improve > > > metadata operations performance of a single client, while maintening > > > the consistency of MDT reply reconstruction and MDT recovery > > > mechanisms. > > > > > > It allows to manage the number of modify RPCs in flight within > > > the client obd structure and to assign a virtual index (the tag) to > > > each modify RPC to help server side cleaning of reply data. > > > > > > The mdc component uses this feature to send multiple modify RPCs > > > in parallel. > > > > Is this a new feature? Why should we take this now and not just wait > > until the code is out of staging? > > Yes on the server side. So the problem on our meta data servers couldn't > handle writing mulitiple bits of data to the back end disk at ths same > time. > > One client side the issue was the metadata operations were being > serialized by a mutex in the MDC layer. That is what this patch fixed. > So for the client it would be a performance improvement patch. So, it's a "performance" patch, which isn't functionality, so why should we merge this to staging now? Why aren't people working on the known coding issues to get this out of staging instead of working on performance stuff? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Patch procedure
Here is how I am going about making the patches. It is basically
what I have picked up from kernel newbies among other sites
and videos on making patches. I would be greatful for any
pointers on what seems to be the problem(s) with why it does
not produce a proper patch.
I will use drivers/staging/xgifb/vb_setmode.c as the example
as it was one of my latest and largest.
I run:
./scripts/checkpatch.pl --terse --strict --file
drivers/staging/xgifb/vb_setmode.c
Patch submissions needed
CHECK: spaces preferred around that '+' (ctx:VxV) 1
CHECK: Alignment should match open parenthesis 9
CHECK: No space is necessary after a cast 7
CHECK: Logical continuations should be on the previous line 1
WARNING: braces {} are not necessary for single statement blocks1
Spelling1
0 errors, 2 warnings, 538 checks, 5526 lines checked
I count each occurance for each type of warning/check I will work on
and figure how many patches will be needed for each.
i.e 90 CHECK: Alignment should match open parenthesis comes to 9 patches.
This example would be 20 total patches in the series.
I check for spelling errors to add a patch or not.
I check to see if non checkpatch type fixes are needed.
i.e. columns/comments need to aligned
I check the mailing list to see if the same type if patches have been
submitted in the last year.
I make all edits needed for one issue
(save Alignment should match open parenthesis for last as other changes
will effect it.)
I open git gui, select rescan, then add hunks to a size that is not
too big and edit the subject, commit message and signed off by.
(It is easier to see how long the patch will be from the hunks and how
many more I have left to fit into the allotted patch series.)
[STYLE 1/20]staging:vb_setmode.c Align to parenthesis
Make suggested modification from checkpatch in reference
to: CHECK: Alignment should match open parenthesis
Signed-off-by: Walt Feasel
Commit it
perl scripts/get_maintainer.pl --nogit-fallback --norolestats
-f drivers/staging/xgifb/vb_setmode.c
Arnaud Patard
Greg Kroah-Hartman
de...@driverdev.osuosl.org
linux-ker...@vger.kernel.org
I also check the TODO list to see if there are any others that
need to copied for the patches.
(The very first patches I used had a option that was removing
the mailing list for some but not others. I dont know why.
it was just what I had copied from a site telling me how to
get the emails to send too.)
git send-email --annotate HEAD^ --to arnaud.pat...@rtp-net.org
--cc gre...@linuxfoundation.org -cc de...@driverdev.osuosl.org
--cc linux-ker...@vger.kernel.org
Verify the message info and press ctrl-x
Send this email? ([y]es|[n]o|[q]uit|[a]ll): y
repeat for remaining hunks.
When all hunks are done start next issue.
I will remove [STYLE] (thought it would make easier to skip if
you did not have time to review just style changes).
New subject example would be:
staging: xgifb: vb_setmode.c Align to parenthesis
In the beginning I made all of the edits at one time per file,
now I only do one type cause I cant separate if two different
issues are part of the same hunk.
The numbering of sequence I will add leading 0 when needed in the
[PATCH v2 09/20]
New subject line would read as:
[PATCH v2 09/20] staging: xgifb: vb_setmode.c Align to parenthesis
The subject lines are the same for the same type of fix except for
the series number.
The patch would be too large if I sent them all in one. I don't know
how else you would want me to name them when there may be 9 patches
for the same issue to keep file size/length down.
Walt
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: lustre: mdc: manage number of modify RPCs in flight
> On Thu, Nov 10, 2016 at 10:51:13AM -0500, James Simmons wrote: > > From: Gregoire Pichon > > > > This patch is the main client part of a new feature that supports > > multiple modify metadata RPCs in parallel. Its goal is to improve > > metadata operations performance of a single client, while maintening > > the consistency of MDT reply reconstruction and MDT recovery > > mechanisms. > > > > It allows to manage the number of modify RPCs in flight within > > the client obd structure and to assign a virtual index (the tag) to > > each modify RPC to help server side cleaning of reply data. > > > > The mdc component uses this feature to send multiple modify RPCs > > in parallel. > > Is this a new feature? Why should we take this now and not just wait > until the code is out of staging? Yes on the server side. So the problem on our meta data servers couldn't handle writing mulitiple bits of data to the back end disk at ths same time. One client side the issue was the metadata operations were being serialized by a mutex in the MDC layer. That is what this patch fixed. So for the client it would be a performance improvement patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: lustre: mdc: manage number of modify RPCs in flight
On Thu, Nov 10, 2016 at 10:51:13AM -0500, James Simmons wrote: > From: Gregoire Pichon > > This patch is the main client part of a new feature that supports > multiple modify metadata RPCs in parallel. Its goal is to improve > metadata operations performance of a single client, while maintening > the consistency of MDT reply reconstruction and MDT recovery > mechanisms. > > It allows to manage the number of modify RPCs in flight within > the client obd structure and to assign a virtual index (the tag) to > each modify RPC to help server side cleaning of reply data. > > The mdc component uses this feature to send multiple modify RPCs > in parallel. Is this a new feature? Why should we take this now and not just wait until the code is out of staging? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/35] second batch of missing lustre 2.8 patches
On Thu, Nov 10, 2016 at 12:30:30PM -0500, James Simmons wrote: > More fixes missing from the upstream client. Also a nice cleanup > with the removal of cl_req which is no longer needed. More cleanup > for lustre_idl.h which is a uapi header. Like the last batch these > patches are independent of order. I didn't apply a few of these (string parsing stuff, and build breakages.) What's the plan for getting this out of staging? I feel like you all are still adding new features along with these "cleanups". Normally that's fine, but I really really want to get this out of staging as it's been there for way too long. When is that going to happen? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 32/35] staging: lustre: mount: fix lmd_parse() to handle commas in expr_list
On Thu, Nov 10, 2016 at 12:31:02PM -0500, James Simmons wrote:
> From: Jian Yu
>
> The lmd_parse() function parses mount options with comma as
> delimiter without considering commas in expr_list as follows
> is a valid LNET nid range syntax:
>
> :== '[' [ ',' ] ']'
>
> This patch fixes the above issue by using cfs_parse_nidlist()
> to parse nid range list instead of using class_parse_nid_quiet()
> to parse only one nid.
ugh, parsing mount strings in the kernel in odd ways, what could ever go
wrong...
>
> Signed-off-by: Jian Yu
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5690
> Reviewed-on: http://review.whamcloud.com/17036
> Reviewed-by: Niu Yawei
> Reviewed-by: Bob Glossman
> Reviewed-by: Oleg Drokin
> Signed-off-by: James Simmons
> ---
> drivers/staging/lustre/lustre/obdclass/obd_mount.c | 91
> ++--
> 1 files changed, 85 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> index 2283e92..1eb8e71 100644
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -871,6 +871,87 @@ static int lmd_parse_mgs(struct lustre_mount_data *lmd,
> char **ptr)
> return 0;
> }
>
> +/**
> + * Find the first comma delimiter from the specified \a buf and make \a *endh
> + * point to the string starting with the comma. The commas in expression list
> + * [...] will be skipped.
> + *
> + * \param[in] bufa comma-separated string
> + * \param[in] endh a pointer to a pointer that will point to the string
> + * starting with the comma
Please drop this mess of \param, it's not needed and is not kernel-doc
format.
> + *
> + * \retval 0 if comma delimiter is found
> + * \retval 1 if comma delimiter is not found
> + */
> +static int lmd_find_comma(char *buf, char **endh)
> +{
> + char *c = buf;
> + int skip = 0;
> +
> + if (!buf)
> + return 1;
> +
> + while (*c != '\0') {
> + if (*c == '[')
> + skip++;
> + else if (*c == ']')
> + skip--;
> +
> + if (*c == ',' && !skip) {
> + if (endh)
> + *endh = c;
> + return 0;
> + }
> + c++;
> + }
> + return 1;
> +}
Don't we have a standard string search function for finding a string in
a string already in the kernel? Why write another one?
Or better yet, why are you using such a crazy string in the first place
from userspace?
Please fix this up and resend.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16' preferred over 'int16_t'
> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Monday, November 14, 2016 4:06 AM
> To: Stuart Yoder
> Cc: Shiva Kerdel ; de...@driverdev.osuosl.org;
> gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; Nipun Gupta ;
> tred...@nvidia.com; Laurentiu Tudor
>
> Subject: Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16'
> preferred over 'int16_t'
>
> On Fri, Nov 11, 2016 at 02:52:31PM +, Stuart Yoder wrote:
> > > > diff --git a/drivers/staging/fsl-mc/include/mc-bus.h
> > > > b/drivers/staging/fsl-mc/include/mc-bus.h
> > > > index e915574..c7cad87 100644
> > > > --- a/drivers/staging/fsl-mc/include/mc-bus.h
> > > > +++ b/drivers/staging/fsl-mc/include/mc-bus.h
> > > > @@ -42,8 +42,8 @@ struct msi_domain_info;
> > > > */
> > > > struct fsl_mc_resource_pool {
> > > > enum fsl_mc_pool_type type;
> > > > - int16_t max_count;
> > > > - int16_t free_count;
> > > > + s16 max_count;
> > >
> > > My understanding is that this has to be signed because the design of
> > > this driver is that we keep adding devices until the the counter
> > > overflows. After that there are a couple tests for
> > > "if (WARN_ON(res_pool->max_count < 0)) " which prevent the driver from
> > > working again.
> > >
> > > This all seems pretty horrible.
> >
> > Can you elaborate?
> >
> > The resource pools managed by this driver are populated by hardware objects
> > discovered when the fsl-mc bus probes a DPRC/container.
> >
> > The number of potential objects discovered of a given type is in the
> > hundreds,
> > so a signed 16-bit number is order of magnitudes larger than anything we
> > will
> > ever encounter.
> >
> > Would you feel better about this if max_count was an int?
>
> Yeah.
>
> >
> > The max_count reflects the total number of objects discovered. If that is
> > exceeded we display a warning, because something is horribly wrong. Nothing
> > stops working, the allocator simply refuses to add anything else to the
> > free list.
>
> I didn't look at this carefully... Anyway we can't remove devices
> either. If we just had an upper bound instead of overflowing the s16
> then we could still remove devices.
>
> >
> > The only reason max_count is there at all is as an internal check against
> > bugs and resource leaks. If the driver is being removed and a resource
> > pool is being freed, max_count must be zero...i.e. all objects should have
> > been removed. If not, there is a leak somewhere. So, it's a sanity check.
> >
>
> Just use a normal upper bound with a #define instead of an magic number
> hidden and then disguised as an integer overflow.
Ok, agree that it would be clearer like that.
Shiva, can you respin this patch and just make both max_count and free_count
to be of type "int".
I will get Dan's suggestion sent as a separate patch...to #define the upper
bound
instead of relying on integer overflow.
Thanks,
Stuart
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 31/35] staging: lustre: obdclass: add export for lprocfs_stats_alloc_one()
On Thu, Nov 10, 2016 at 12:31:01PM -0500, James Simmons wrote: > From: Chennaiah Palla > > When compiling the kernel without optimization, when using GCOV, > the lprocfs_stats_alloc_one() symbol is not properly exported to > other modules and causes the ptlrpc module to fail loading with > an unknown symbol. Added EXPORT_SYMBOL(lprocfs_stats_alloc_one) > so that this works properly. No, let's fix this properly. Please get rid of lprocfs_stats_lock(), or if you _really_ need it, move it to a .c file. Having it in a .h file is just a mess, as this proves. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wlang-ng: Fix block comments style warnings in prism2sta.h
On Fri, Nov 11, 2016 at 02:13:06PM +0100, Timo Schmid wrote: > This patch fixes the following checkpatch.pl warning in prism2sta.h: > WARNING: Block comments should align the * on each line > > No more warnings block comments warnings for this file. > > Signed-off-by: Timo Schmid > --- > drivers/staging/wlan-ng/prism2sta.c | 36 ++-- > 1 file changed, 18 insertions(+), 18 deletions(-) > This patch doesn't apply at all to my tree, are you sure you made it against linux-next? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192e: Fix checkpatch warnings
On Thu, Nov 10, 2016 at 10:31:32PM +0530, Yamanappagouda Patil wrote: > Fixed checkpatch.pl warning on Block comments. > > Signed-off-by: Yamanappagouda Patil > --- Both of your patches did different things, yet you had the same subject line :( Please fix them up and resend with unique subjects (hint, adding ".pl" doesn't make it unique... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: Remove explicit NULL pointer
On Fri, Nov 11, 2016 at 05:46:45PM +0530, Maninder Singh wrote: > Replace direct comparisons to NULL i.e. > 'x == NULL' with '!x' > 'x != NULL' with 'x' > > Signed-off-by: Maninder Singh A patch that was sent right before yours causes this patch to now have conflicts. Can you refresh it against my staging-testing branch and resend? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] Staging: vme: Use BIT macro for bit field definitions.
On Thu, Nov 10, 2016 at 10:17:23PM +0300, Dan Carpenter wrote: > On Wed, Nov 02, 2016 at 01:27:09AM +0300, Anton Leshchenko wrote: > > #define PIO2_CNTR_MODE00 > > -#define PIO2_CNTR_MODE1(1 << 1) > > -#define PIO2_CNTR_MODE2(2 << 1) > > -#define PIO2_CNTR_MODE3(3 << 1) > > -#define PIO2_CNTR_MODE4(4 << 1) > > -#define PIO2_CNTR_MODE5(5 << 1) > > +#define PIO2_CNTR_MODE1BIT(1) > > +#define PIO2_CNTR_MODE2BIT(2) > > +#define PIO2_CNTR_MODE3(BIT(1) | BIT(2)) > > +#define PIO2_CNTR_MODE4BIT(4) > > This should be BIT(3) Ugh, there were other parts wrong with this now that I look closer. I've now reverted it. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] [STYLE]staging:braille:braille_console.c [1/2]
On Thu, Nov 10, 2016 at 08:12:07PM -0500, Walt Feasel wrote:
> Made suggested modifications from checkpatch in reference
> to ERROR: Use 4 digit octal (0777) not decimal permissions
>
> Signed-off-by: Walt Feasel
> ---
> drivers/accessibility/braille/braille_console.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
{sigh}
Ok, I count over a hundred different patches from you in my inbox for
review. That's great, but you need to redo them so that I can review
them.
First off, your Subject: lines are a bit odd. You don't need the
"[STYLE]" portion, and you move around the [1/2] from the front to the
end. Also, you aren't using git to number them, so they go from 9/12 to
10/12, which does not sort (it should be 09/12).
So please, use git to create the patch series.
Look at other patches sent to the lists for how to get good subjects,
don't use the same subject for multiple patches, that is an easy grounds
for rejection, and please put space characters after ':'.
Also say what you are doing in the subject, you don't do that here at
all.
So, for example, this Subject should be:
[PATCH] staging: speakup: braille_console.c: use octal mode values
Read the file, Documentation/SubmittingPatches for a good description of
how to make a good subject.
Second, your patches are not threaded at all. If you use 'git
send-email' they will all be linked up properly, making it much easier
for me to apply them, and for others to just delete them when they don't
care about the patch series.
So use 'git send-email' please.
And when using git send-email, send out patches in batches for a
specific driver, don't send out multiple series for the same driver, as
then I don't know which to accept or not.
Thirdly, please properly cc: the maintainers and developers of the
drivers you are modifying. Use 'scripts/get_maintainers.pl' to do this.
You seem to do it in some places, but not all, so it's hard to know what
is going on there.
And lastly, slow down a bit. Try all of this out on a single patch
series first. Once you get them accepted, then keep going, and work on
more. I don't want to see you waste energy on things that don't get
accepted (like all of these patches).
Does this help?
Right now I'm going to drop all of the patches you sent me, so please,
work on the above things and start to slowly resend them, in the proper
format, so that I can accept them and merge them to the kernel tree,
which is what we both want to see happen.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] [STYLE]staging:skein:threefish_block.c remove blank lines
On Mon, Nov 14, 2016 at 01:34:15PM +0300, Dan Carpenter wrote: > Please just delete the blank lines. You aren't that special like a snow > flake. I agree, they need to be removed so that we don't have people waste time on this, and the code gets closer to the correct kernel style rules. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] [STYLE]staging:xgifb:XGI_main.h Align columns
Your subjects are still wrong... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] [STYLE]staging:skein:threefish_block.c remove blank lines
Please just delete the blank lines. You aren't that special like a snow flake. regards, dan carpetner ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Revert "[STYLE]staging:MAINTAINERS email revision speakup"
This is not at all how revert commits work. Anyway, no one applied the original patch. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] [STYLE 1/2]staging:speakup:i18n.c Block align on *
These patches are ok, but the subjects are all wrong. Please fix and resend. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16' preferred over 'int16_t'
On Fri, Nov 11, 2016 at 02:52:31PM +, Stuart Yoder wrote:
> > > diff --git a/drivers/staging/fsl-mc/include/mc-bus.h
> > > b/drivers/staging/fsl-mc/include/mc-bus.h
> > > index e915574..c7cad87 100644
> > > --- a/drivers/staging/fsl-mc/include/mc-bus.h
> > > +++ b/drivers/staging/fsl-mc/include/mc-bus.h
> > > @@ -42,8 +42,8 @@ struct msi_domain_info;
> > > */
> > > struct fsl_mc_resource_pool {
> > > enum fsl_mc_pool_type type;
> > > - int16_t max_count;
> > > - int16_t free_count;
> > > + s16 max_count;
> >
> > My understanding is that this has to be signed because the design of
> > this driver is that we keep adding devices until the the counter
> > overflows. After that there are a couple tests for
> > "if (WARN_ON(res_pool->max_count < 0)) " which prevent the driver from
> > working again.
> >
> > This all seems pretty horrible.
>
> Can you elaborate?
>
> The resource pools managed by this driver are populated by hardware objects
> discovered when the fsl-mc bus probes a DPRC/container.
>
> The number of potential objects discovered of a given type is in the hundreds,
> so a signed 16-bit number is order of magnitudes larger than anything we will
> ever encounter.
>
> Would you feel better about this if max_count was an int?
Yeah.
>
> The max_count reflects the total number of objects discovered. If that is
> exceeded we display a warning, because something is horribly wrong. Nothing
> stops working, the allocator simply refuses to add anything else to the
> free list.
I didn't look at this carefully... Anyway we can't remove devices
either. If we just had an upper bound instead of overflowing the s16
then we could still remove devices.
>
> The only reason max_count is there at all is as an internal check against
> bugs and resource leaks. If the driver is being removed and a resource
> pool is being freed, max_count must be zero...i.e. all objects should have
> been removed. If not, there is a leak somewhere. So, it's a sanity check.
>
Just use a normal upper bound with a #define instead of an magic number
hidden and then disguised as an integer overflow.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vc04_services: rework ioctl code path
On Thu, Nov 10, 2016 at 10:15:31PM -0800, Michael Zoran wrote:
> +static void *
> +vchiq_ioctl_kmalloc(struct vchiq_ioctl_call_context *ctxt, size_t size)
> +{
> + void *mem;
> +
> + if (!ctxt->stackmem_used && size < sizeof(ctxt->stackmem)) {
> + ctxt->stackmem_used = true;
> + return ctxt->stackmem;
> + }
> +
> + mem = kmalloc(size + sizeof(void *), GFP_KERNEL);
This is a potential integer overflow leading to corruption. I don't
understand why we need this complicated memory management anyway...
> + if (!mem)
> + return NULL;
> +
> + *(void **)mem = ctxt->prev_kmalloc;
> + ctxt->prev_kmalloc = mem;
> +
> + return mem + sizeof(void *);
> +}
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE]staging:xgifb:XGI_main.h Align columns
Make alignment changes to tabular data for consistency
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main.h | 52
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main.h b/drivers/staging/xgifb/XGI_main.h
index 85079fe..c1e0b6b 100644
--- a/drivers/staging/xgifb/XGI_main.h
+++ b/drivers/staging/xgifb/XGI_main.h
@@ -174,7 +174,7 @@ static const struct _XGI_tvtype {
{"NTSC",2},
{"pal", 1},
{"ntsc",2},
- {"\0", -1}
+ {"\0", -1}
};
static const struct _XGI_vrate {
@@ -183,44 +183,44 @@ static const struct _XGI_vrate {
u16 yres;
u16 refresh;
} XGIfb_vrate[] = {
- {1, 640, 480, 60}, {2, 640, 480, 72},
- {3, 640, 480, 75}, {4, 640, 480, 85},
+ {1, 640, 480, 60}, {2, 640, 480, 72},
+ {3, 640, 480, 75}, {4, 640, 480, 85},
- {5, 640, 480, 100}, {6, 640, 480, 120},
- {7, 640, 480, 160}, {8, 640, 480, 200},
+ {5, 640, 480, 100}, {6, 640, 480, 120},
+ {7, 640, 480, 160}, {8, 640, 480, 200},
- {1, 720, 480, 60},
- {1, 720, 576, 58},
- {1, 800, 480, 60}, {2, 800, 480, 75}, {3, 800, 480, 85},
- {1, 800, 600, 60}, {2, 800, 600, 72}, {3, 800, 600, 75},
- {4, 800, 600, 85}, {5, 800, 600, 100},
- {6, 800, 600, 120}, {7, 800, 600, 160},
+ {1, 720, 480, 60},
+ {1, 720, 576, 58},
+ {1, 800, 480, 60}, {2, 800, 480, 75}, {3, 800, 480, 85},
+ {1, 800, 600, 60}, {2, 800, 600, 72}, {3, 800, 600, 75},
+ {4, 800, 600, 85}, {5, 800, 600, 100},
+ {6, 800, 600, 120}, {7, 800, 600, 160},
{1, 1024, 768, 60}, {2, 1024, 768, 70}, {3, 1024, 768, 75},
- {4, 1024, 768, 85}, {5, 1024, 768, 100}, {6, 1024, 768, 120},
- {1, 1024, 576, 60}, {2, 1024, 576, 75}, {3, 1024, 576, 85},
- {1, 1024, 600, 60},
- {1, 1152, 768, 60},
- {1, 1280, 720, 60}, {2, 1280, 720, 75}, {3, 1280, 720, 85},
- {1, 1280, 768, 60},
+ {4, 1024, 768, 85}, {5, 1024, 768, 100}, {6, 1024, 768, 120},
+ {1, 1024, 576, 60}, {2, 1024, 576, 75}, {3, 1024, 576, 85},
+ {1, 1024, 600, 60},
+ {1, 1152, 768, 60},
+ {1, 1280, 720, 60}, {2, 1280, 720, 75}, {3, 1280, 720, 85},
+ {1, 1280, 768, 60},
{1, 1280, 1024, 60}, {2, 1280, 1024, 75}, {3, 1280, 1024, 85},
- {1, 1280, 960, 70},
- {1, 1400, 1050, 60},
- {1, 1600, 1200, 60}, {2, 1600, 1200, 65},
+ {1, 1280, 960, 70},
+ {1, 1400, 1050, 60},
+ {1, 1600, 1200, 60}, {2, 1600, 1200, 65},
{3, 1600, 1200, 70}, {4, 1600, 1200, 75},
- {5, 1600, 1200, 85}, {6, 1600, 1200, 100},
+ {5, 1600, 1200, 85}, {6, 1600, 1200, 100},
{7, 1600, 1200, 120},
- {1, 1920, 1440, 60}, {2, 1920, 1440, 65},
+ {1, 1920, 1440, 60}, {2, 1920, 1440, 65},
{3, 1920, 1440, 70}, {4, 1920, 1440, 75},
- {5, 1920, 1440, 85}, {6, 1920, 1440, 100},
- {1, 2048, 1536, 60}, {2, 2048, 1536, 65},
+ {5, 1920, 1440, 85}, {6, 1920, 1440, 100},
+ {1, 2048, 1536, 60}, {2, 2048, 1536, 65},
{3, 2048, 1536, 70}, {4, 2048, 1536, 75},
- {5, 2048, 1536, 85},
- {0, 0, 0, 0}
+ {5, 2048, 1536, 85},
+ {0,0,0, 0}
};
static const struct _XGI_TV_filter {
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] Staging: ks7010: Prefer using the BIT macro
Replace all occurences of (1< --- * Subject made more clear * Remove unnecessary indentation drivers/staging/ks7010/ks7010_sdio.h | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.h b/drivers/staging/ks7010/ks7010_sdio.h index c89e570..0a0951f 100644 --- a/drivers/staging/ks7010/ks7010_sdio.h +++ b/drivers/staging/ks7010/ks7010_sdio.h @@ -53,14 +53,14 @@ /* ARM to SD interrupt Pending */ #define INT_PENDING0x24 -#define INT_GCR_B (1<<7) -#define INT_GCR_A (1<<6) -#define INT_WRITE_STATUS (1<<5) -#define INT_WRITE_INDEX(1<<4) -#define INT_WRITE_SIZE (1<<3) -#define INT_READ_STATUS(1<<2) -#define INT_READ_INDEX (1<<1) -#define INT_READ_SIZE (1<<0) +#define INT_GCR_B BIT(7) +#define INT_GCR_A BIT(6) +#define INT_WRITE_STATUS BIT(5) +#define INT_WRITE_INDEXBIT(4) +#define INT_WRITE_SIZE BIT(3) +#define INT_READ_STATUSBIT(2) +#define INT_READ_INDEX BIT(1) +#define INT_READ_SIZE BIT(0) /* General Communication Register A */ #define GCR_A 0x28 -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] Subject: Staging: ks7010: Prefer using the BIT macro
On Mon, Nov 14, 2016 at 02:33:01PM +0530, Punit Vara wrote: > Replace all occurences of (1< ks7010_sdio.h to get rid of checkpatch.pl "CHECK" output "Prefer > using BIT macro". > > Signed-off-by: Punit Vara > --- > * remove unecessary indentation Your "Subject:" looks a bit odd now :( 3rd time's a charm? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: ks7010: Prefer using the BIT macro
Sorry for that. I have updated the patch and sent you version 2. If I am still wrong could you please share me link to the sample commit ? thanks, PV On Mon, Nov 14, 2016 at 12:30 PM, Greg KH wrote: > On Mon, Nov 14, 2016 at 12:20:40PM +0530, Punit Vara wrote: >> Replace all occurances of (1<> ks7010_sdio.h to get rid of checkpatch.pl "CHECK" output "Prefer using >> BIT macro" >> >> Signed-off-by: Punit Vara > > Why the odd indentation? > > Same for the Subject: line, why the extra whitespace? > > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] Subject: Staging: ks7010: Prefer using the BIT macro
Replace all occurences of (1< --- * remove unecessary indentation drivers/staging/ks7010/ks7010_sdio.h | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.h b/drivers/staging/ks7010/ks7010_sdio.h index c89e570..0a0951f 100644 --- a/drivers/staging/ks7010/ks7010_sdio.h +++ b/drivers/staging/ks7010/ks7010_sdio.h @@ -53,14 +53,14 @@ /* ARM to SD interrupt Pending */ #define INT_PENDING0x24 -#define INT_GCR_B (1<<7) -#define INT_GCR_A (1<<6) -#define INT_WRITE_STATUS (1<<5) -#define INT_WRITE_INDEX(1<<4) -#define INT_WRITE_SIZE (1<<3) -#define INT_READ_STATUS(1<<2) -#define INT_READ_INDEX (1<<1) -#define INT_READ_SIZE (1<<0) +#define INT_GCR_B BIT(7) +#define INT_GCR_A BIT(6) +#define INT_WRITE_STATUS BIT(5) +#define INT_WRITE_INDEXBIT(4) +#define INT_WRITE_SIZE BIT(3) +#define INT_READ_STATUSBIT(2) +#define INT_READ_INDEX BIT(1) +#define INT_READ_SIZE BIT(0) /* General Communication Register A */ #define GCR_A 0x28 -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 9/9]staging:xgifb:XGI_main_26.c Align on parenthesis
Make suggested modification from checkpatch in reference
to: CHECK: Alignment should match open parenthesis
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 37 -
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 96936e3..05ff19a 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -1616,8 +1616,7 @@ static int __init XGIfb_setup(char *options)
return 0;
}
-static int xgifb_probe(struct pci_dev *pdev,
- const struct pci_device_id *ent)
+static int xgifb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
u8 reg, reg1;
u8 CR48, CR38;
@@ -1722,7 +1721,7 @@ static int xgifb_probe(struct pci_dev *pdev,
xgifb_info->video_size,
"XGIfb FB")) {
dev_err(&pdev->dev, "Unable request memory size %x\n",
- xgifb_info->video_size);
+ xgifb_info->video_size);
dev_err(&pdev->dev,
"Fatal error: Unable to reserve frame buffer memory. Is
there another framebuffer driver active?\n");
ret = -ENODEV;
@@ -1885,8 +1884,7 @@ static int xgifb_probe(struct pci_dev *pdev,
xgifb_info->refresh_rate = refresh_rate;
if (xgifb_info->refresh_rate == 0)
xgifb_info->refresh_rate = 60;
- if (XGIfb_search_refresh_rate(xgifb_info,
- xgifb_info->refresh_rate) == 0) {
+ if (XGIfb_search_refresh_rate(xgifb_info, xgifb_info->refresh_rate) ==
0) {
xgifb_info->rate_idx = 1;
xgifb_info->refresh_rate = 60;
}
@@ -1920,16 +1918,13 @@ static int xgifb_probe(struct pci_dev *pdev,
break;
default:
xgifb_info->video_cmap_len = 16;
- pr_info("Unsupported depth %d\n",
- xgifb_info->video_bpp);
+ pr_info("Unsupported depth %d\n", xgifb_info->video_bpp);
break;
}
- pr_info("Default mode is %dx%dx%d (%dHz)\n",
- xgifb_info->video_width,
- xgifb_info->video_height,
- xgifb_info->video_bpp,
- xgifb_info->refresh_rate);
+ pr_info("Default mode is %dx%dx%d (%dHz)\n", xgifb_info->video_width,
+ xgifb_info->video_height, xgifb_info->video_bpp,
+ xgifb_info->refresh_rate);
fb_info->var.red.length = 8;
fb_info->var.green.length = 8;
@@ -1952,15 +1947,15 @@ static int xgifb_probe(struct pci_dev *pdev,
XGIbios_mode[xgifb_info->mode_idx].mode_no));
if (XGIfb_mode_rate_to_ddata(&xgifb_info->dev_info, hw_info,
- XGIbios_mode[xgifb_info->mode_idx].mode_no,
- &fb_info->var.left_margin,
- &fb_info->var.right_margin,
- &fb_info->var.upper_margin,
- &fb_info->var.lower_margin,
- &fb_info->var.hsync_len,
- &fb_info->var.vsync_len,
- &fb_info->var.sync,
- &fb_info->var.vmode)) {
+XGIbios_mode[xgifb_info->mode_idx].mode_no,
+&fb_info->var.left_margin,
+&fb_info->var.right_margin,
+&fb_info->var.upper_margin,
+&fb_info->var.lower_margin,
+&fb_info->var.hsync_len,
+&fb_info->var.vsync_len,
+&fb_info->var.sync,
+&fb_info->var.vmode)) {
if ((fb_info->var.vmode & FB_VMODE_MASK) ==
FB_VMODE_INTERLACED) {
fb_info->var.yres <<= 1;
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 8/9]staging:xgifb:XGI_main_26.c Align on parenthesis
Make suggested modification from checkpatch in reference
to: CHECK: Alignment should match open parenthesis
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 34fc5ce..96936e3 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -1156,7 +1156,7 @@ static int XGIfb_setcolreg(unsigned int regno, unsigned
int red,
/* --- FBDev related routines for all series -- */
static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,
- struct fb_info *info)
+struct fb_info *info)
{
struct xgifb_video_info *xgifb_info = info->par;
@@ -1250,10 +1250,10 @@ static int XGIfb_check_var(struct fb_var_screeninfo
*var, struct fb_info *info)
search_idx = 0;
while ((XGIbios_mode[search_idx].mode_no != 0) &&
- (XGIbios_mode[search_idx].xres <= var->xres)) {
+ (XGIbios_mode[search_idx].xres <= var->xres)) {
if ((XGIbios_mode[search_idx].xres == var->xres) &&
- (XGIbios_mode[search_idx].yres == var->yres) &&
- (XGIbios_mode[search_idx].bpp == var->bits_per_pixel)) {
+ (XGIbios_mode[search_idx].yres == var->yres) &&
+ (XGIbios_mode[search_idx].bpp == var->bits_per_pixel)) {
if (XGIfb_validate_mode(xgifb_info, search_idx) > 0) {
found_mode = 1;
break;
@@ -1263,8 +1263,8 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
}
if (!found_mode) {
- pr_err("%dx%dx%d is no valid mode\n",
- var->xres, var->yres, var->bits_per_pixel);
+ pr_err("%dx%dx%d is no valid mode\n", var->xres, var->yres,
+ var->bits_per_pixel);
search_idx = 0;
while (XGIbios_mode[search_idx].mode_no != 0) {
if ((var->xres <= XGIbios_mode[search_idx].xres) &&
@@ -1282,12 +1282,12 @@ static int XGIfb_check_var(struct fb_var_screeninfo
*var, struct fb_info *info)
if (found_mode) {
var->xres = XGIbios_mode[search_idx].xres;
var->yres = XGIbios_mode[search_idx].yres;
- pr_debug("Adapted to mode %dx%dx%d\n",
- var->xres, var->yres, var->bits_per_pixel);
+ pr_debug("Adapted to mode %dx%dx%d\n", var->xres,
+var->yres, var->bits_per_pixel);
} else {
pr_err("Failed to find similar mode to %dx%dx%d\n",
- var->xres, var->yres, var->bits_per_pixel);
+ var->xres, var->yres, var->bits_per_pixel);
return -EINVAL;
}
}
@@ -1318,8 +1318,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
return 0;
}
-static int XGIfb_pan_display(struct fb_var_screeninfo *var,
- struct fb_info *info)
+static int XGIfb_pan_display(struct fb_var_screeninfo *var, struct fb_info
*info)
{
int err;
@@ -1459,7 +1458,7 @@ static int XGIfb_get_dram_size(struct xgifb_video_info
*xgifb_info)
xgifb_info->video_size = xgifb_info->video_size * ChannelNum;
pr_info("SR14=%x DramSzie %x ChannelNum %x\n",
- reg,
+ reg,
xgifb_info->video_size, ChannelNum);
return 0;
}
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 7/9]staging:xgifb:XGI_main_26.c Align on parenthesis
Make suggested modification from checkpatch in reference
to: CHECK: Alignment should match open parenthesis
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 29 +
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 58d9a10..34fc5ce 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -56,8 +56,8 @@ static inline void dumpVGAReg(struct xgifb_video_info
*xgifb_info)
/* --- Hardware Access Routines -- */
static int XGIfb_mode_rate_to_dclock(struct vb_device_info *XGI_Pr,
- struct xgi_hw_device_info *HwDeviceExtension,
- unsigned char modeno)
+struct xgi_hw_device_info
*HwDeviceExtension,
+unsigned char modeno)
{
unsigned short ModeNo = modeno;
unsigned short ModeIdIndex = 0, ClockIndex = 0;
@@ -68,7 +68,7 @@ static int XGIfb_mode_rate_to_dclock(struct vb_device_info
*XGI_Pr,
XGI_SearchModeID(ModeNo, &ModeIdIndex);
RefreshRateTableIndex = XGI_GetRatePtrCRT2(HwDeviceExtension, ModeNo,
- ModeIdIndex, XGI_Pr);
+ ModeIdIndex, XGI_Pr);
ClockIndex = XGI330_RefIndex[RefreshRateTableIndex].Ext_CRTVCLK;
@@ -76,11 +76,11 @@ static int XGIfb_mode_rate_to_dclock(struct vb_device_info
*XGI_Pr,
}
static int XGIfb_mode_rate_to_ddata(struct vb_device_info *XGI_Pr,
- struct xgi_hw_device_info *HwDeviceExtension,
- unsigned char modeno,
- u32 *left_margin, u32 *right_margin, u32 *upper_margin,
- u32 *lower_margin, u32 *hsync_len, u32 *vsync_len, u32 *sync,
- u32 *vmode)
+ struct xgi_hw_device_info
*HwDeviceExtension,
+ unsigned char modeno, u32 *left_margin,
+ u32 *right_margin, u32 *upper_margin,
+ u32 *lower_margin, u32 *hsync_len,
+ u32 *vsync_len, u32 *sync, u32 *vmode)
{
unsigned short ModeNo = modeno;
unsigned short ModeIdIndex, index = 0;
@@ -95,7 +95,7 @@ static int XGIfb_mode_rate_to_ddata(struct vb_device_info
*XGI_Pr,
if (!XGI_SearchModeID(ModeNo, &ModeIdIndex))
return 0;
RefreshRateTableIndex = XGI_GetRatePtrCRT2(HwDeviceExtension, ModeNo,
- ModeIdIndex, XGI_Pr);
+ ModeIdIndex, XGI_Pr);
index = XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC;
sr_data = XGI_CRT1Table[index].CR[5];
@@ -682,7 +682,7 @@ static void XGIfb_pre_setmode(struct xgifb_video_info
*xgifb_info)
xgifb_reg_set(XGICR, IND_XGI_SCRATCH_REG_CR30, cr30);
xgifb_reg_set(XGICR, IND_XGI_SCRATCH_REG_CR31, cr31);
xgifb_reg_set(XGICR, IND_XGI_SCRATCH_REG_CR33,
- (xgifb_info->rate_idx & 0x0F));
+ (xgifb_info->rate_idx & 0x0F));
}
static void XGIfb_post_setmode(struct xgifb_video_info *xgifb_info)
@@ -904,7 +904,7 @@ static void XGIfb_post_setmode(struct xgifb_video_info
*xgifb_info)
}
static int XGIfb_do_set_var(struct fb_var_screeninfo *var, int isactive,
- struct fb_info *info)
+ struct fb_info *info)
{
struct xgifb_video_info *xgifb_info = info->par;
struct xgi_hw_device_info *hw_info = &xgifb_info->hw_info;
@@ -943,11 +943,8 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var,
int isactive,
xgifb_info->refresh_rate = 60;
}
- pr_debug("Change mode to %dx%dx%d-%dHz\n",
- var->xres,
- var->yres,
- var->bits_per_pixel,
- xgifb_info->refresh_rate);
+ pr_debug("Change mode to %dx%dx%d-%dHz\n", var->xres, var->yres,
+var->bits_per_pixel, xgifb_info->refresh_rate);
old_mode = xgifb_info->mode_idx;
xgifb_info->mode_idx = 0;
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 6/9]staging:xgifb:XGI_main_26.c No space after cast
Make suggested modification from checkpatch in reference
to: CHECK: No space is necessary after a cast
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 9f0708f..58d9a10 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -1141,7 +1141,7 @@ static int XGIfb_setcolreg(unsigned int regno, unsigned
int red,
}
break;
case 16:
- ((u32 *) (info->pseudo_palette))[regno] = ((red & 0xf800))
+ ((u32 *)(info->pseudo_palette))[regno] = ((red & 0xf800))
| ((green & 0xfc00) >> 5) | ((blue & 0xf800)
>> 11);
break;
@@ -1149,7 +1149,7 @@ static int XGIfb_setcolreg(unsigned int regno, unsigned
int red,
red >>= 8;
green >>= 8;
blue >>= 8;
- ((u32 *) (info->pseudo_palette))[regno] = (red << 16) | (green
+ ((u32 *)(info->pseudo_palette))[regno] = (red << 16) | (green
<< 8) | (blue);
break;
}
@@ -1241,7 +1241,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
drate = 10 / var->pixclock;
hrate = (drate * 1000) / htotal;
xgifb_info->refresh_rate =
- (unsigned int) (hrate * 2 / vtotal);
+ (unsigned int)(hrate * 2 / vtotal);
pr_debug(
"%s: pixclock = %d ,htotal=%d, vtotal=%d\n"
"%s: drate=%d, hrate=%d, refresh_rate=%d\n",
@@ -1656,7 +1656,7 @@ static int xgifb_probe(struct pci_dev *pdev,
xgifb_info->mmio_size = pci_resource_len(pdev, 1);
xgifb_info->vga_base = pci_resource_start(pdev, 2) + 0x30;
dev_info(&pdev->dev, "Relocate IO address: %Lx [%08lx]\n",
-(u64) pci_resource_start(pdev, 2),
+(u64)pci_resource_start(pdev, 2),
xgifb_info->vga_base);
if (pci_enable_device(pdev)) {
@@ -1749,13 +1749,13 @@ static int xgifb_probe(struct pci_dev *pdev,
dev_info(&pdev->dev,
"Framebuffer at 0x%Lx, mapped to 0x%p, size %dk\n",
-(u64) xgifb_info->video_base,
+(u64)xgifb_info->video_base,
xgifb_info->video_vbase,
xgifb_info->video_size / 1024);
dev_info(&pdev->dev,
"MMIO at 0x%Lx, mapped to 0x%p, size %ldk\n",
-(u64) xgifb_info->mmio_base, xgifb_info->mmio_vbase,
+(u64)xgifb_info->mmio_base, xgifb_info->mmio_vbase,
xgifb_info->mmio_size / 1024);
pci_set_drvdata(pdev, xgifb_info);
@@ -1950,7 +1950,7 @@ static int xgifb_probe(struct pci_dev *pdev,
XGIfb_bpp_to_var(xgifb_info, &fb_info->var);
- fb_info->var.pixclock = (u32) (10 /
+ fb_info->var.pixclock = (u32)(10 /
XGIfb_mode_rate_to_dclock(&xgifb_info->dev_info,
hw_info,
XGIbios_mode[xgifb_info->mode_idx].mode_no));
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 5/9]staging:xgifb:XGI_main_26.c No space after cast
Make suggested modification from checkpatch in reference
to: CHECK: No space is necessary after a cast
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 06d098e..9f0708f 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -105,7 +105,7 @@ static int XGIfb_mode_rate_to_ddata(struct vb_device_info
*XGI_Pr,
cr_data = XGI_CRT1Table[index].CR[3];
/* Horizontal retrace (=sync) start */
- HRS = (cr_data & 0xff) | ((unsigned short) (sr_data & 0xC0) << 2);
+ HRS = (cr_data & 0xff) | ((unsigned short)(sr_data & 0xC0) << 2);
F = HRS - HDE - 3;
sr_data = XGI_CRT1Table[index].CR[6];
@@ -115,8 +115,8 @@ static int XGIfb_mode_rate_to_ddata(struct vb_device_info
*XGI_Pr,
cr_data2 = XGI_CRT1Table[index].CR[4];
/* Horizontal blank end */
- HBE = (cr_data & 0x1f) | ((unsigned short) (cr_data2 & 0x80) >> 2)
- | ((unsigned short) (sr_data & 0x03) << 6);
+ HBE = (cr_data & 0x1f) | ((unsigned short)(cr_data2 & 0x80) >> 2)
+ | ((unsigned short)(sr_data & 0x03) << 6);
/* Horizontal retrace (=sync) end */
HRE = (cr_data2 & 0x1f) | ((sr_data & 0x04) << 3);
@@ -142,15 +142,15 @@ static int XGIfb_mode_rate_to_ddata(struct vb_device_info
*XGI_Pr,
cr_data = XGI_CRT1Table[index].CR[10];
/* Vertical retrace (=sync) start */
- VRS = (cr_data & 0xff) | ((unsigned short) (cr_data2 & 0x04) << 6)
- | ((unsigned short) (cr_data2 & 0x80) << 2)
- | ((unsigned short) (sr_data & 0x08) << 7);
+ VRS = (cr_data & 0xff) | ((unsigned short)(cr_data2 & 0x04) << 6)
+ | ((unsigned short)(cr_data2 & 0x80) << 2)
+ | ((unsigned short)(sr_data & 0x08) << 7);
F = VRS + 1 - VDE;
cr_data = XGI_CRT1Table[index].CR[13];
/* Vertical blank end */
- VBE = (cr_data & 0xff) | ((unsigned short) (sr_data & 0x10) << 4);
+ VBE = (cr_data & 0xff) | ((unsigned short)(sr_data & 0x10) << 4);
temp = VBE - ((VDE - 1) & 511);
B = (temp > 0) ? temp : (temp + 512);
@@ -937,7 +937,7 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var,
int isactive,
if (var->pixclock) {
drate = 10 / var->pixclock;
hrate = (drate * 1000) / htotal;
- xgifb_info->refresh_rate = (unsigned int) (hrate * 2
+ xgifb_info->refresh_rate = (unsigned int)(hrate * 2
/ vtotal);
} else {
xgifb_info->refresh_rate = 60;
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 4/9]staging:xgifb:XGI_main_26.c Logical continuations
Make suggested modification from checkpatch in reference
to: CHECK: Logical continuations should be on the previous line
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index ceeb4a8..06d098e 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -231,11 +231,11 @@ static int XGIfb_GetXG21DefaultLVDSModeIdx(struct
xgifb_video_info *xgifb_info)
{
int i = 0;
- while ((XGIbios_mode[i].mode_no != 0)
- && (XGIbios_mode[i].xres <= xgifb_info->lvds_data.LVDSHDE)) {
- if ((XGIbios_mode[i].xres == xgifb_info->lvds_data.LVDSHDE)
- && (XGIbios_mode[i].yres == xgifb_info->lvds_data.LVDSVDE)
- && (XGIbios_mode[i].bpp == 8)) {
+ while ((XGIbios_mode[i].mode_no != 0) &&
+ (XGIbios_mode[i].xres <= xgifb_info->lvds_data.LVDSHDE)) {
+ if ((XGIbios_mode[i].xres == xgifb_info->lvds_data.LVDSHDE) &&
+ (XGIbios_mode[i].yres == xgifb_info->lvds_data.LVDSVDE) &&
+ (XGIbios_mode[i].bpp == 8)) {
return i;
}
i++;
@@ -384,9 +384,8 @@ static int XGIfb_validate_mode(struct xgifb_video_info
*xgifb_info, int myindex)
return -1;
break;
case 640:
- if ((XGIbios_mode[myindex].yres != 400)
- && (XGIbios_mode[myindex].yres
- != 480))
+ if ((XGIbios_mode[myindex].yres != 400) &&
+ (XGIbios_mode[myindex].yres != 480))
return -1;
break;
case 800:
@@ -1335,9 +1334,8 @@ static int XGIfb_pan_display(struct fb_var_screeninfo
*var,
if (var->vmode & FB_VMODE_YWRAP) {
if (var->yoffset >= info->var.yres_virtual || var->xoffset)
return -EINVAL;
- } else if (var->xoffset + info->var.xres > info->var.xres_virtual
- || var->yoffset + info->var.yres
- > info->var.yres_virtual) {
+ } else if (var->xoffset + info->var.xres > info->var.xres_virtual ||
+ var->yoffset + info->var.yres > info->var.yres_virtual) {
return -EINVAL;
}
err = XGIfb_pan_var(var, info);
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 3/9]staging:xgifb:XGI_main_26.c Spaces preferred around
Make suggested modification from checkpatch in reference
to: CHECK: spaces preferred around that '-,&'
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index e1dfcfe..ceeb4a8 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -562,7 +562,7 @@ static u8 XGIfb_search_refresh_rate(struct xgifb_video_info
*xgifb_info,
!= 1)) {
pr_debug("Adjusting rate from %d down
to %d\n",
rate,
-XGIfb_vrate[i-1].refresh);
+XGIfb_vrate[i - 1].refresh);
xgifb_info->rate_idx =
XGIfb_vrate[i - 1].idx;
xgifb_info->refresh_rate =
@@ -1686,7 +1686,7 @@ static int xgifb_probe(struct pci_dev *pdev,
case PCI_DEVICE_ID_XGI_20:
xgifb_reg_or(XGICR, Index_CR_GPIO_Reg3, GPIOG_EN);
CR48 = xgifb_reg_get(XGICR, Index_CR_GPIO_Reg1);
- if (CR48&GPIOG_READ)
+ if (CR48 & GPIOG_READ)
xgifb_info->chip = XG21;
else
xgifb_info->chip = XG20;
@@ -1772,9 +1772,9 @@ static int xgifb_probe(struct pci_dev *pdev,
xgifb_info->hasVB = HASVB_NONE;
} else if (xgifb_info->chip == XG21) {
CR38 = xgifb_reg_get(XGICR, 0x38);
- if ((CR38&0xE0) == 0xC0)
+ if ((CR38 & 0xE0) == 0xC0)
xgifb_info->display2 = XGIFB_DISP_LCD;
- else if ((CR38&0xE0) == 0x60)
+ else if ((CR38 & 0xE0) == 0x60)
xgifb_info->hasVB = HASVB_CHRONTEL;
else
xgifb_info->hasVB = HASVB_NONE;
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 2/9]staging:xgifb:XGI_main_26.c Remove blank lines after {
Make suggested modification from checkpatch in reference
to: CHECK: Blank lines aren't necessary after an open brace '{'
Signed-off-by: Walt Feasel
---
drivers/staging/xgifb/XGI_main_26.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 7bd4e5a..e1dfcfe 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -730,7 +730,6 @@ static void XGIfb_post_setmode(struct xgifb_video_info
*xgifb_info)
if (xgifb_info->display2 == XGIFB_DISP_TV &&
xgifb_info->hasVB == HASVB_301) {
-
reg = xgifb_reg_get(XGIPART4, 0x01);
if (reg < 0xB0) { /* Set filter for XGI301 */
@@ -763,16 +762,13 @@ static void XGIfb_post_setmode(struct xgifb_video_info
*xgifb_info)
0x01);
if (xgifb_info->TV_type == TVMODE_NTSC) {
-
xgifb_reg_and(XGIPART2, 0x3a, 0x1f);
if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
-
xgifb_reg_and(XGIPART2, 0x30, 0xdf);
} else if (xgifb_info->TV_plug
== TVPLUG_COMPOSITE) {
-
xgifb_reg_or(XGIPART2, 0x30, 0x20);
switch (xgifb_info->video_width) {
@@ -822,16 +818,13 @@ static void XGIfb_post_setmode(struct xgifb_video_info
*xgifb_info)
}
} else if (xgifb_info->TV_type == TVMODE_PAL) {
-
xgifb_reg_and(XGIPART2, 0x3A, 0x1F);
if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
-
xgifb_reg_and(XGIPART2, 0x30, 0xDF);
} else if (xgifb_info->TV_plug
== TVPLUG_COMPOSITE) {
-
xgifb_reg_or(XGIPART2, 0x30, 0x20);
switch (xgifb_info->video_width) {
@@ -992,7 +985,6 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var,
int isactive,
}
if (isactive) {
-
XGIfb_pre_setmode(xgifb_info);
if (XGISetModeNew(xgifb_info, hw_info,
XGIbios_mode[xgifb_info->mode_idx].mode_no)
@@ -1275,7 +1267,6 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
}
if (!found_mode) {
-
pr_err("%dx%dx%d is no valid mode\n",
var->xres, var->yres, var->bits_per_pixel);
search_idx = 0;
@@ -1401,7 +1392,6 @@ static struct fb_ops XGIfb_ops = {
static int XGIfb_get_dram_size(struct xgifb_video_info *xgifb_info)
{
-
u8 ChannelNum, tmp;
u8 reg = 0;
@@ -1596,7 +1586,6 @@ static int __init XGIfb_setup(char *options)
pr_info("Options: %s\n", options);
while ((this_opt = strsep(&options, ",")) != NULL) {
-
if (!*this_opt)
continue;
@@ -1978,7 +1967,6 @@ static int xgifb_probe(struct pci_dev *pdev,
&fb_info->var.vsync_len,
&fb_info->var.sync,
&fb_info->var.vmode)) {
-
if ((fb_info->var.vmode & FB_VMODE_MASK) ==
FB_VMODE_INTERLACED) {
fb_info->var.yres <<= 1;
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [STYLE 1/9]staging:xgifb:XGI_main_26.c Remove blank lines before }
Make suggested modification from checkpatch in reference to: CHECK: Blank lines aren't necessary before a close brace '}' Signed-off-by: Walt Feasel --- drivers/staging/xgifb/XGI_main_26.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c index 982f90f..7bd4e5a 100644 --- a/drivers/staging/xgifb/XGI_main_26.c +++ b/drivers/staging/xgifb/XGI_main_26.c @@ -1477,7 +1477,6 @@ static int XGIfb_get_dram_size(struct xgifb_video_info *xgifb_info) reg, xgifb_info->video_size, ChannelNum); return 0; - } static void XGIfb_detect_VB(struct xgifb_video_info *xgifb_info) @@ -1990,7 +1989,6 @@ static int xgifb_probe(struct pci_dev *pdev, fb_info->var.yres >>= 1; fb_info->var.yres_virtual >>= 1; } - } fb_info->flags = FBINFO_FLAG_DEFAULT; -- 2.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
