RE: [PATCH v2] hv: hv_fcopy: drop the obsolete message on transfer failure
> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Thursday, November 27, 2014 15:15 PM
> To: Dexuan Cui
> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
> Srinivasan; vkuzn...@redhat.com; Haiyang Zhang
> Subject: Re: [PATCH v2] hv: hv_fcopy: drop the obsolete message on transfer
> failure
> - Original Message -
> > In the case the user-space daemon crashes, hangs or is killed, we
> > need to down the semaphore, otherwise, after the daemon starts next
> > time, the obsolete data in fcopy_transaction.message or
> > fcopy_transaction.fcopy_msg will be used immediately.
> >
> > Reviewed-by: Vitaly Kuznetsov
> > Cc: K. Y. Srinivasan
> > Signed-off-by: Dexuan Cui
> > ---
> >
> > v2: I removed the "FCP" prefix as Greg asked.
> >
> > I also updated the output message a little:
> > "FCP: failed to acquire the semaphore" -->
> > "can not acquire the semaphore: it is benign"
> >
> > drivers/hv/hv_fcopy.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
> > index 23b2ce2..c518ad9 100644
> > --- a/drivers/hv/hv_fcopy.c
> > +++ b/drivers/hv/hv_fcopy.c
> > @@ -86,6 +86,15 @@ static void fcopy_work_func(struct work_struct
> *dummy)
> > * process the pending transaction.
> > */
> > fcopy_respond_to_host(HV_E_FAIL);
> > +
> > + /* In the case the user-space daemon crashes, hangs or is killed, we
> > +* need to down the semaphore, otherwise, after the daemon starts
> next
> > +* time, the obsolete data in fcopy_transaction.message or
> > +* fcopy_transaction.fcopy_msg will be used immediately.
> > +*/
>
> Looks still racy, what happens if the daemon start before down_trylock()
> but after fcopy_respont_to_host() here?
Jason,
Thanks for pointing this out!
IMO we can resolve this by adding down_trylock() in fcopy_release().
What's your opinion?
>
> > + if (down_trylock(&fcopy_transaction.read_sema))
> > + pr_debug("can not acquire the semaphore: it is benign\n");
>
> typo
> > +
> > }
Sorry -- what typo do you mean?
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] hv: hv_fcopy: drop the obsolete message on transfer failure
On Thu, Nov 27, 2014 at 4:50 PM, Dexuan Cui wrote:
-Original Message-
From: Jason Wang [mailto:jasow...@redhat.com]
Sent: Thursday, November 27, 2014 15:15 PM
To: Dexuan Cui
Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
driverdev-
de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
Srinivasan; vkuzn...@redhat.com; Haiyang Zhang
Subject: Re: [PATCH v2] hv: hv_fcopy: drop the obsolete message on
transfer
failure
- Original Message -
> In the case the user-space daemon crashes, hangs or is killed, we
> need to down the semaphore, otherwise, after the daemon starts
next
> time, the obsolete data in fcopy_transaction.message or
> fcopy_transaction.fcopy_msg will be used immediately.
>
> Reviewed-by: Vitaly Kuznetsov
> Cc: K. Y. Srinivasan
> Signed-off-by: Dexuan Cui
> ---
>
> v2: I removed the "FCP" prefix as Greg asked.
>
> I also updated the output message a little:
> "FCP: failed to acquire the semaphore" -->
> "can not acquire the semaphore: it is benign"
>
> drivers/hv/hv_fcopy.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
> index 23b2ce2..c518ad9 100644
> --- a/drivers/hv/hv_fcopy.c
> +++ b/drivers/hv/hv_fcopy.c
> @@ -86,6 +86,15 @@ static void fcopy_work_func(struct work_struct
*dummy)
>* process the pending transaction.
>*/
> fcopy_respond_to_host(HV_E_FAIL);
> +
> + /* In the case the user-space daemon crashes, hangs or is
killed, we
> + * need to down the semaphore, otherwise, after the daemon
starts
next
> + * time, the obsolete data in fcopy_transaction.message or
> + * fcopy_transaction.fcopy_msg will be used immediately.
> + */
Looks still racy, what happens if the daemon start before
down_trylock()
but after fcopy_respont_to_host() here?
Jason,
Thanks for pointing this out!
IMO we can resolve this by adding down_trylock() in fcopy_release().
What's your opinion?
Looks better and need to cancel the timeout also here?
> + if (down_trylock(&fcopy_transaction.read_sema))
> + pr_debug("can not acquire the semaphore: it is benign\n");
typo
> +
> }
Sorry -- what typo do you mean?
s/benign/begin/ ?
Thanks,
-- Dexuan
�NrybXǧv^){.n+{zXܨ}Ơz&j:+vzZ++zfh~iz w?&)ߢf^jǫym@Aa 0hi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers: hv: vmbus: prevent cpu offlining on newer hypervisors
Dexuan Cui writes:
>> -Original Message-
>> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
>> Behalf Of Greg Kroah-Hartman
>> Sent: Thursday, November 27, 2014 11:03 AM
>> To: Vitaly Kuznetsov
>> Cc: de...@linuxdriverproject.org; Haiyang Zhang; linux-
>> ker...@vger.kernel.org
>> Subject: Re: [PATCH] Drivers: hv: vmbus: prevent cpu offlining on newer
>> hypervisors
>>
>> On Wed, Nov 26, 2014 at 02:52:22PM +0100, Vitaly Kuznetsov wrote:
>> > When an SMP Hyper-V guest is running on top of 2012R2 Server and
>> secondary
>> > cpus are sent offline (with echo 0 >
>> /sys/devices/system/cpu/cpu$cpu/online)
>> > the system freeze is observed. This happens due to the fact that on newer
>> > hypervisors (Win8, WS2012R2, ...) vmbus channel handlers are
>> distributed
>> > across all cpus (see init_vp_index() function in
>> drivers/hv/channel_mgmt.c)
>> > and on cpu offlining nobody reassigns them to CPU0. Prevent cpu
>> offlining
>> > when vmbus is loaded until the issue is fixed host-side.
>> >
>> > This patch also disables hibernation but it is OK as it is also broken (MCE
>> > error is hit on resume). Suspend still works.
>> >
>> > Tested with WS2008R2 and WS2012R2.
>> >
>> > Signed-off-by: Vitaly Kuznetsov
>> > ---
>> > drivers/hv/vmbus_drv.c | 19 +++
>> > 1 file changed, 19 insertions(+)
>> >
>> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> > index 4d6b269..9a82249 100644
>> > --- a/drivers/hv/vmbus_drv.c
>> > +++ b/drivers/hv/vmbus_drv.c
>> > @@ -32,6 +32,7 @@
>> > #include
>> > #include
>> > #include
>> > +#include
>> > #include
>> > #include
>> > #include
>> > @@ -671,6 +672,13 @@ static void vmbus_isr(void)
>> >tasklet_schedule(&msg_dpc);
>> > }
>> >
>> > +#ifdef CONFIG_HOTPLUG_CPU
>> > +static int hyperv_cpu_disable(void)
>> > +{
>> > + return -1;
>> > +}
>> > +#endif
>> > +
>> > /*
>> > * vmbus_bus_init -Main vmbus driver initialization routine.
>> > *
>> > @@ -711,6 +719,12 @@ static int vmbus_bus_init(int irq)
>> >if (ret)
>> >goto err_alloc;
>> >
>> > +#ifdef CONFIG_HOTPLUG_CPU
>> > + if ((vmbus_proto_version != VERSION_WS2008) &&
>> > + (vmbus_proto_version != VERSION_WIN7))
>> > + smp_ops.cpu_disable = hyperv_cpu_disable;
>> > +#endif
>> > +
>> >vmbus_request_offers();
>> >
>> >return 0;
>> > @@ -964,6 +978,11 @@ static void __exit vmbus_exit(void)
>> >bus_unregister(&hv_bus);
>> >hv_cleanup();
>> >acpi_bus_unregister_driver(&vmbus_acpi_driver);
>> > +#ifdef CONFIG_HOTPLUG_CPU
>> > + if ((vmbus_proto_version != VERSION_WS2008) &&
>> > + (vmbus_proto_version != VERSION_WIN7))
>> > + smp_ops.cpu_disable = native_cpu_disable;
>> > +#endif
>> > }
>>
>> #ifdef in a .c file is not a good idea to do if at all possible, please
>> only put this in one place, using a function call to "hide" the mess.
>>
>> greg k-h
>
> Hi Vitaly,
> The idea of the patch is good to me.
>
> I agree with Greg.
> BTW, maybe hv_cpu_hotplug_quirk() is a better name?
My idea was that eventually this function will start doing something
real (e.g. switching channels to cpu0 if it doesn't happen fully
host-side) so I called it with a general name 'hyperv_cpu_disable'.
I'll try addressing our and Greg's comments in v2, thanks!
>
> Thanks,
> -- Dexuan
--
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart
`interrupt_pci9118_ai_dma()` is called on interrupt to transfer data
from DMA buffers into the comedi async data buffer. Currently it always
restarts DMA. If double buffering, it restarts DMA on the next DMA
buffer before processing the current DMA buffer, otherwise it restarts
DMA on the same DMA buffer after it has been processed.
For single buffering we can avoid restarting the DMA transfer by
checking the async event flags after the current buffer has been
processed, which is easy.
For double buffering, we need to know how many valid samples there are
in the current buffer before it has been processed and determine whether
there is enough to complete the acquisition. Call new function
`valid_samples_in_act_dma_buf()` to determine the number of valid
samples in the current DMA buffer, and compare that with the result of
`comedi_nsamples_left()` to determine if DMA needs to be restarted.
(`comedi_nsamples_left()` needs an upper bound to clamp to, so use the
number of valid samples in the DMA buffer plus one for our test.)
It is still possible for DMA to be restarted unnecessarily in the double
buffer case if a `COMEDI_CB_OVERFLOW` event occurs while copying to the
comedi async buffer, but it doesn't really matter. The ongoing DMA
operation will get disabled when the subdevice's `cancel()` handler is
called when the events are handled later in the interrupt service
routine (as it does currently).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/adl_pci9118.c | 78 +---
1 file changed, 71 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index aecfae8..c0ea733 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -446,6 +446,62 @@ static void interrupt_pci9118_ai_mode4_switch(struct
comedi_device *dev,
outl(devpriv->ai_cfg, dev->iobase + PCI9118_AI_CFG_REG);
}
+static unsigned int valid_samples_in_act_dma_buf(struct comedi_device *dev,
+struct comedi_subdevice *s,
+unsigned int n_raw_samples)
+{
+ struct pci9118_private *devpriv = dev->private;
+ struct comedi_cmd *cmd = &s->async->cmd;
+ unsigned int start_pos = devpriv->ai_add_front;
+ unsigned int stop_pos = start_pos + cmd->chanlist_len;
+ unsigned int span_len = stop_pos + devpriv->ai_add_back;
+ unsigned int dma_pos = devpriv->ai_act_dmapos;
+ unsigned int whole_spans, n_samples, x;
+
+ if (span_len == cmd->chanlist_len)
+ return n_raw_samples; /* use all samples */
+
+ /*
+* Not all samples are to be used. Buffer contents consist of a
+* possibly non-whole number of spans and a region of each span
+* is to be used.
+*
+* Account for samples in whole number of spans.
+*/
+ whole_spans = n_raw_samples / span_len;
+ n_samples = whole_spans * cmd->chanlist_len;
+ n_raw_samples -= whole_spans * span_len;
+
+ /*
+* Deal with remaining samples which could overlap up to two spans.
+*/
+ while (n_raw_samples) {
+ if (dma_pos < start_pos) {
+ /* Skip samples before start position. */
+ x = start_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ dma_pos += x;
+ n_raw_samples -= x;
+ if (!n_raw_samples)
+ break;
+ }
+ if (dma_pos < stop_pos) {
+ /* Include samples before stop position. */
+ x = stop_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ n_samples += x;
+ dma_pos += x;
+ n_raw_samples -= x;
+ }
+ /* Advance to next span. */
+ start_pos += span_len;
+ stop_pos += span_len;
+ }
+ return n_samples;
+}
+
static unsigned int defragment_dma_buffer(struct comedi_device *dev,
struct comedi_subdevice *s,
unsigned short *dma_buffer,
@@ -607,10 +663,16 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
struct pci9118_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
- unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
+ unsigned int n_all = comedi_bytes_to_samples(s, dmabuf->use_size);
+ unsigned int n_valid;
+ bool more_dma;
+
+ /* determine whether more DMA b
[PATCH 0/3] staging: comedi: adl_pci9118: some dma transfer changes
For streaming acquisition on the analog input subdevice, this driver normally uses DMA double buffering into two internal DMA buffers, so it can switch buffers early after a DMA transfer has completed, while it processes the completed DMA buffer. PATCH 1 is just a bit of tidy up. PATCH 2 avoids switching the DMA double buffer at the end of acquisition. For DMA single buffering, it avoids restarting the DMA transfer if acquisition has ended normally or due to a comedi buffer overflow error. (However, for double buffering, the DMA transfer has already been started on the other DMA buffer and will be stopped soon after.) PATCH 3 eliminates a possible defragmentation step in the DMA buffer contents before they are transferred to the comedi buffer. Instead, the DMA buffer fragments are copied directly to the comedi buffer, or is copied all in one go if the data is not fragmented. This work is in response to Hartley's "PATCH 4/4] staging: comedi: adl_pci9118: switch DMA buffers after writing samples", dated Mon, 10 Nov 2014 1959:17 -0500, message ID <1415667477-28403-5-git-send-email-hswee...@visionengravers.com>, which was not applied. These patches are against Greg's "staging-testing" branch. 1) staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a bit 2) staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart 3) staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation step drivers/staging/comedi/drivers/adl_pci9118.c | 157 +-- 1 file changed, 121 insertions(+), 36 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a bit
Eliminate the `next_dma_buf` variable in `interrupt_pci9118_ai_dma()`.
It holds the next value of `devpriv->dma_actbuf` when double buffering
is used, but we can just set that to the next value directly at the
point where the buffers are switched as the old value is not used
anywhere else.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/adl_pci9118.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index 5e0ff9d..aecfae8 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -608,16 +608,15 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
struct comedi_cmd *cmd = &s->async->cmd;
struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
- unsigned int next_dma_buf;
- if (devpriv->dma_doublebuf) { /*
-* switch DMA buffers if is used
-* double buffering
-*/
- next_dma_buf = 1 - devpriv->dma_actbuf;
- pci9118_amcc_setup_dma(dev, next_dma_buf);
- if (devpriv->ai_do == 4)
- interrupt_pci9118_ai_mode4_switch(dev, next_dma_buf);
+ /* switch DMA buffers and restart DMA if double buffering */
+ if (devpriv->dma_doublebuf) {
+ devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
+ pci9118_amcc_setup_dma(dev, devpriv->dma_actbuf);
+ if (devpriv->ai_do == 4) {
+ interrupt_pci9118_ai_mode4_switch(dev,
+ devpriv->dma_actbuf);
+ }
}
if (nsamples) {
@@ -631,11 +630,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
s->async->events |= COMEDI_CB_EOA;
}
- if (devpriv->dma_doublebuf) {
- /* switch dma buffers */
- devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
- } else {
- /* restart DMA if is not used double buffering */
+ /* restart DMA if not double buffering */
+ if (!devpriv->dma_doublebuf) {
pci9118_amcc_setup_dma(dev, 0);
if (devpriv->ai_do == 4)
interrupt_pci9118_ai_mode4_switch(dev, 0);
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation step
The DMA operations used by the driver may have been set up to acquire
data from unwanted channels in addition to the wanted channels.
Currently, `interrupt_pci9118_ai_dma()` calls `defragment_dma_buffer()`
to move all the wanted data to the start of the DMA buffer and then
calls `comedi_buf_write_samples()` to copy it all to the comedi async
buffer. Those two functions used to be called from
`move_block_from_dma()` which was absorbed into
`interrupt_pci9118_ai_dma()`.
Reinstate `move_block_from_dma()` but rewrite it to copy data directly
from the wanted fragments of the DMA buffer to the comedi async buffer
without defragmenting the buffer first.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/adl_pci9118.c | 67 +++-
1 file changed, 46 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index c0ea733..2660358 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -502,29 +502,56 @@ static unsigned int valid_samples_in_act_dma_buf(struct
comedi_device *dev,
return n_samples;
}
-static unsigned int defragment_dma_buffer(struct comedi_device *dev,
- struct comedi_subdevice *s,
- unsigned short *dma_buffer,
- unsigned int num_samples)
+static void move_block_from_dma(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ unsigned short *dma_buffer,
+ unsigned int n_raw_samples)
{
struct pci9118_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- unsigned int i = 0, j = 0;
- unsigned int start_pos = devpriv->ai_add_front,
- stop_pos = devpriv->ai_add_front + cmd->chanlist_len;
- unsigned int raw_scanlen = devpriv->ai_add_front + cmd->chanlist_len +
- devpriv->ai_add_back;
+ unsigned int start_pos = devpriv->ai_add_front;
+ unsigned int stop_pos = start_pos + cmd->chanlist_len;
+ unsigned int span_len = stop_pos + devpriv->ai_add_back;
+ unsigned int dma_pos = devpriv->ai_act_dmapos;
+ unsigned int x;
- for (i = 0; i < num_samples; i++) {
- if (devpriv->ai_act_dmapos >= start_pos &&
- devpriv->ai_act_dmapos < stop_pos) {
- dma_buffer[j++] = dma_buffer[i];
+ if (span_len == cmd->chanlist_len) {
+ /* All samples are to be copied. */
+ comedi_buf_write_samples(s, dma_buffer, n_raw_samples);
+ dma_pos += n_raw_samples;
+ } else {
+ /*
+* Not all samples are to be copied. Buffer contents consist
+* of a possibly non-whole number of spans and a region of
+* each span is to be copied.
+*/
+ while (n_raw_samples) {
+ if (dma_pos < start_pos) {
+ /* Skip samples before start position. */
+ x = start_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ dma_pos += x;
+ n_raw_samples -= x;
+ if (!n_raw_samples)
+ break;
+ }
+ if (dma_pos < stop_pos) {
+ /* Copy samples before stop position. */
+ x = stop_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ comedi_buf_write_samples(s, dma_buffer, x);
+ dma_pos += x;
+ n_raw_samples -= x;
+ }
+ /* Advance to next span. */
+ start_pos += span_len;
+ stop_pos += span_len;
}
- devpriv->ai_act_dmapos++;
- devpriv->ai_act_dmapos %= raw_scanlen;
}
-
- return j;
+ /* Update position in span for next time. */
+ devpriv->ai_act_dmapos = dma_pos % span_len;
}
static void pci9118_exttrg_enable(struct comedi_device *dev, bool enable)
@@ -681,10 +708,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
}
}
- if (n_all) {
- n_valid = defragment_dma_buffer(dev, s, dmabuf->virt, n_all);
- comedi_buf_write_samples(s, dmabuf->virt, n_valid);
- }
+ if (n_all)
+ move_block_from_dma(dev, s, dmabuf->virt, n_all);
if (!devpriv->ai_neverending) {
if
RE: [PATCH v2] hv: hv_fcopy: drop the obsolete message on transfer failure
> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Thursday, November 27, 2014 17:01 PM
> To: Dexuan Cui
> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
> Srinivasan; vkuzn...@redhat.com; Haiyang Zhang
> Subject: RE: [PATCH v2] hv: hv_fcopy: drop the obsolete message on transfer
> failure
> On Thu, Nov 27, 2014 at 4:50 PM, Dexuan Cui wrote:
> >> -Original Message-
> >> From: Jason Wang [mailto:jasow...@redhat.com]
> >> Sent: Thursday, November 27, 2014 15:15 PM
> >> To: Dexuan Cui
> >> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> >> driverdev-
> >> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
> >> Srinivasan; vkuzn...@redhat.com; Haiyang Zhang
> >> Subject: Re: [PATCH v2] hv: hv_fcopy: drop the obsolete message on
> >> transfer
> >> failure
> >> - Original Message -
> >> > In the case the user-space daemon crashes, hangs or is killed, we
> >> > need to down the semaphore, otherwise, after the daemon starts
> >> next
> >> > time, the obsolete data in fcopy_transaction.message or
> >> > fcopy_transaction.fcopy_msg will be used immediately.
> >> >
> >> > Reviewed-by: Vitaly Kuznetsov
> >> > Cc: K. Y. Srinivasan
> >> > Signed-off-by: Dexuan Cui
> >> > ---
> >> >
> >> > v2: I removed the "FCP" prefix as Greg asked.
> >> >
> >> > I also updated the output message a little:
> >> > "FCP: failed to acquire the semaphore" -->
> >> > "can not acquire the semaphore: it is benign"
> >> >
> >> > drivers/hv/hv_fcopy.c | 9 +
> >> > 1 file changed, 9 insertions(+)
> >> >
> >> > diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
> >> > index 23b2ce2..c518ad9 100644
> >> > --- a/drivers/hv/hv_fcopy.c
> >> > +++ b/drivers/hv/hv_fcopy.c
> >> > @@ -86,6 +86,15 @@ static void fcopy_work_func(struct work_struct
> >> *dummy)
> >> > * process the pending transaction.
> >> > */
> >> > fcopy_respond_to_host(HV_E_FAIL);
> >> > +
> >> > + /* In the case the user-space daemon crashes, hangs or is
> >> killed, we
> >> > +* need to down the semaphore, otherwise, after the daemon
> >> starts
> >> next
> >> > +* time, the obsolete data in fcopy_transaction.message or
> >> > +* fcopy_transaction.fcopy_msg will be used immediately.
> >> > +*/
> >>
> >> Looks still racy, what happens if the daemon start before
> >> down_trylock()
> >> but after fcopy_respont_to_host() here?
> > Jason,
> > Thanks for pointing this out!
> > IMO we can resolve this by adding down_trylock() in fcopy_release().
> > What's your opinion?
>
>
> Looks better and need to cancel the timeout also here?
OK, let me post a v3.
> >
> >
> >>
> >> > + if (down_trylock(&fcopy_transaction.read_sema))
> >> > + pr_debug("can not acquire the semaphore: it is
> >> benign\n");
> >>
> >> typo
> >> > +
> >> > }
> > Sorry -- what typo do you mean?
>
> s/benign/begin/ ?
I meant the issue(can't get the semaphore) is benign.
I think we can just remove the message, as KY suggested.
Instead, I'll add a comment for it.
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] hv: hv_fcopy: drop the obsolete message on transfer failure
In the case the user-space daemon crashes, hangs or is killed, we
need to down the semaphore, otherwise, after the daemon starts next
time, the obsolete data in fcopy_transaction.message or
fcopy_transaction.fcopy_msg will be used immediately.
Cc: Jason Wang
Cc: Vitaly Kuznetsov
Cc: K. Y. Srinivasan
Signed-off-by: Dexuan Cui
---
v2: I removed the "FCP" prefix as Greg asked.
I also updated the output message a little:
"FCP: failed to acquire the semaphore" -->
"can not acquire the semaphore: it is benign"
v3: I added the code in fcopy_release() as Jason Wang suggested.
I removed the pr_debug (it isn't so meaningful)and added a comment instead.
drivers/hv/hv_fcopy.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 23b2ce2..faa6ba6 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -86,6 +86,18 @@ static void fcopy_work_func(struct work_struct *dummy)
* process the pending transaction.
*/
fcopy_respond_to_host(HV_E_FAIL);
+
+ /* In the case the user-space daemon crashes, hangs or is killed, we
+* need to down the semaphore, otherwise, after the daemon starts next
+* time, the obsolete data in fcopy_transaction.message or
+* fcopy_transaction.fcopy_msg will be used immediately.
+*
+* NOTE: fcopy_read() happens to get the semaphore (very rare)? We're
+* still OK, because we've reported the failure to the host.
+*/
+ if (down_trylock(&fcopy_transaction.read_sema))
+ ;
+
}
static int fcopy_handle_handshake(u32 version)
@@ -351,6 +363,13 @@ static int fcopy_release(struct inode *inode, struct file
*f)
*/
in_hand_shake = true;
opened = false;
+
+ if (cancel_delayed_work_sync(&fcopy_work)) {
+ /* We haven't up()-ed the semaphore(very rare)? */
+ if (down_trylock(&fcopy_transaction.read_sema))
+ ;
+ fcopy_respond_to_host(HV_E_FAIL);
+ }
return 0;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers:staging:octeon: Fixed checkpatch warning
C V is my initals . Can I just go with the name Athira Lekshmi ? Thanking You Athira Lekshmi C V On Thu, Nov 27, 2014 at 3:14 AM, Greg KH wrote: > On Wed, Nov 26, 2014 at 05:58:37PM +0530, Athira Lekshmi C V wrote: >> Fixed the checkpatch warning: >> WARNING: Missing a blank line after declarations >> >> Signed-off-by: Athira Lekshmi C V > > What is the "C V" at the end of the name here? Is that your "full > name"? I need a real name, not initials. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Wed, Nov 26, 2014 at 01:58:01PM -0800, Greg Kroah-Hartman wrote: > On Wed, Nov 19, 2014 at 09:38:46PM +0100, Mariusz Gorski wrote: > > Avoid magic number and use a comparison with a defined value instead > > that checks whether module param has been set by the user to some > > value at loading time. > > > > Signed-off-by: Mariusz Gorski > > Acked-by: Willy Tarreau > > --- > > v2: Don't introduce new macros for param value check > > > > drivers/staging/panel/panel.c | 86 > > +-- > > 1 file changed, 43 insertions(+), 43 deletions(-) > > Ugh, I messed up here, and applied the first series, which was acked. > > Mariusz, can you resend the patches that I didn't apply, I can't seem to > get the rest of these to work properly. Greg, if I get you here correctly, you've applied all 9 patches from v1 and none from v2, so what you need now is a v1->v2 patch, right? > thanks, > > greg k-h Cheers, Mariusz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: android: ion: One function call less in ion_buffer_create() after error detection
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index df12cc3..7a26b85 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -226,7 +226,7 @@ static struct ion_buffer *ion_buffer_create(struct
>> ion_heap *heap,
>> buffer->pages = vmalloc(sizeof(struct page *) * num_pages);
>> if (!buffer->pages) {
>> ret = -ENOMEM;
>> -goto err1;
>> +goto err2;
>> }
>>
>> for_each_sg(table->sgl, sg, table->nents, i) {
>> @@ -262,7 +262,6 @@ static struct ion_buffer *ion_buffer_create(struct
>> ion_heap *heap,
>> err:
>> heap->ops->unmap_dma(heap, buffer);
>> heap->ops->free(buffer);
>> -err1:
>> vfree(buffer->pages);
>> err2:
>
> Now we have "err" and "err2", which doesn't make much sense,
> please fix up.
How do you want this source code to be improved?
Should I introduce longer names for the affected jump labels?
Regards,
Markus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Drivers:Staging:rtl8188eu:hal:usb_halinit.c: Added blank line after declarations
This patch fixes the five checkpatch.pl warnings:
WARNING:Missing a blank line after declaration
Signed-off-by: Anjana Sasindran
---
drivers/staging/rtl8188eu/hal/usb_halinit.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c
b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index 14650e9..439828c 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -1931,6 +1931,7 @@ GetHalDefVar8188EUsb(
case HW_DEF_RA_INFO_DUMP:
{
u8 entry_id = *((u8 *)pValue);
+
if (check_fwstate(&Adapter->mlmepriv, _FW_LINKED)) {
DBG_88E(" RA status check
===\n");
DBG_88E("Mac_id:%d , RateID = %d, RAUseRate =
0x%08x, RateSGI = %d, DecisionRate = 0x%02x ,PTStage = %d\n",
@@ -1946,6 +1947,7 @@ GetHalDefVar8188EUsb(
case HW_DEF_ODM_DBG_FLAG:
{
struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
+
pr_info("dm_ocm->DebugComponents = 0x%llx\n",
dm_ocm->DebugComponents);
}
break;
@@ -1994,6 +1996,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter,
enum hal_def_variable eV
} else if (dm_func == 6) {/* turn on all dynamic func */
if (!(podmpriv->SupportAbility &
DYNAMIC_BB_DIG)) {
struct rtw_dig *pDigTable =
&podmpriv->DM_DigTable;
+
pDigTable->CurIGValue =
usb_read8(Adapter, 0xc50);
}
podmpriv->SupportAbility =
DYNAMIC_ALL_FUNC_ENABLE;
@@ -2011,6 +2014,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter,
enum hal_def_variable eV
{
u8 bRSSIDump = *((u8 *)pValue);
struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
+
if (bRSSIDump)
dm_ocm->DebugComponents =
ODM_COMP_DIG|ODM_COMP_FA_CNT;
else
@@ -2021,7 +2025,9 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter,
enum hal_def_variable eV
{
u64 DebugComponents = *((u64 *)pValue);
struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
+
dm_ocm->DebugComponents = DebugComponents;
+
}
break;
default:
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: octeon: Fix checkpatch warning
On Wed, Nov 26, 2014 at 06:34:10PM -0800, Greg KH wrote:
> On Thu, Nov 27, 2014 at 12:35:23AM +, Luis de Bethencourt wrote:
> > On Wed, Nov 26, 2014 at 01:45:23PM -0800, Greg KH wrote:
> > > On Tue, Nov 25, 2014 at 01:26:14PM +, Luis de Bethencourt wrote:
> > > > This patch fixes the checkpatch.pl warnings:
> > > >
> > > > WARNING: line over 80 characters
> > > > + int cores_in_use = core_state.baseline_cores -
> > > > atomic_read(&core_state.available_cores);
> > > >
> > > > WARNING: line over 80 characters
> > > > + skb->data = skb->head + work->packet_ptr.s.addr
> > > > - cvmx_ptr_to_phys(skb->head);
> > > >
> > > > Signed-off-by: Luis de Bethencourt
> > > > ---
> > > > drivers/staging/octeon/ethernet-rx.c | 6 --
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/octeon/ethernet-rx.c
> > > > b/drivers/staging/octeon/ethernet-rx.c
> > > > index 44e372f..bd83f55 100644
> > > > --- a/drivers/staging/octeon/ethernet-rx.c
> > > > +++ b/drivers/staging/octeon/ethernet-rx.c
> > > > @@ -295,7 +295,8 @@ static int cvm_oct_napi_poll(struct napi_struct
> > > > *napi, int budget)
> > > > */
> > > > union cvmx_pow_wq_int_cntx counts;
> > > > int backlog;
> > > > - int cores_in_use = core_state.baseline_cores -
> > > > atomic_read(&core_state.available_cores);
> > > > + int cores_in_use = core_state.baseline_cores -
> > > > +
> > > > atomic_read(&core_state.available_cores);
> > > > counts.u64 =
> > > > cvmx_read_csr(CVMX_POW_WQ_INT_CNTX(pow_receive_group));
> > > > backlog = counts.s.iq_cnt + counts.s.ds_cnt;
> > > > if (backlog > budget * cores_in_use && napi !=
> > > > NULL)
> > > > @@ -324,7 +325,8 @@ static int cvm_oct_napi_poll(struct napi_struct
> > > > *napi, int budget)
> > > > * buffer.
> > > > */
> > > > if (likely(skb_in_hw)) {
> > > > - skb->data = skb->head + work->packet_ptr.s.addr
> > > > - cvmx_ptr_to_phys(skb->head);
> > > > + skb->data = skb->head + work->packet_ptr.s.addr
> > > > -
> > > > + cvmx_ptr_to_phys(skb->head);
> > > > prefetch(skb->data);
> > > > skb->len = work->len;
> > > > skb_set_tail_pointer(skb, skb->len);
> > > > --
> > > > 2.1.3
> > >
> > > No longer applies to my tree :(
> >
> > I'm confused.
> >
> > I just tried applying it to what I think is your tree is and it worked.
> > https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/log/?h=staging-next
> >
> > Do I have this wrong?
>
> I'm applying patches first to staging-testing to get some 0-day buildbot
> testing before merging them to staging-next these days, as I've been
> burned with common problems too many times. I took some other octeon
> patches that were sent before yours were that caused the conflict. If
> you look at staging-testing right now you can see that.
>
> Hope this helps,
>
> greg k-h
This is very helpful!
I am about to send a new version of the patch, this time against staging-test.
Thanks for the time to explain this, and sorry about the previos patch not
applying.
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: android: ion: One function call less in ion_buffer_create() after error detection
Am 27.11.2014 15:25, schrieb SF Markus Elfring:
>>> diff --git a/drivers/staging/android/ion/ion.c
>>> b/drivers/staging/android/ion/ion.c
>>> index df12cc3..7a26b85 100644
>>> --- a/drivers/staging/android/ion/ion.c
>>> +++ b/drivers/staging/android/ion/ion.c
>>> @@ -226,7 +226,7 @@ static struct ion_buffer *ion_buffer_create(struct
>>> ion_heap *heap,
>>> buffer->pages = vmalloc(sizeof(struct page *) * num_pages);
>>> if (!buffer->pages) {
>>> ret = -ENOMEM;
>>> - goto err1;
>>> + goto err2;
>>> }
>>>
>>> for_each_sg(table->sgl, sg, table->nents, i) {
>>> @@ -262,7 +262,6 @@ static struct ion_buffer *ion_buffer_create(struct
>>> ion_heap *heap,
>>> err:
>>> heap->ops->unmap_dma(heap, buffer);
>>> heap->ops->free(buffer);
>>> -err1:
>>> vfree(buffer->pages);
>>> err2:
>>
>> Now we have "err" and "err2", which doesn't make much sense,
>> please fix up.
>
> How do you want this source code to be improved?
> Should I introduce longer names for the affected jump labels?
>
hi markus,
the confusion arises because the errX are numbered and now on in the sequence
is missing.
So far i see you can renumber them or give more descriptiv names.
hope that helps,
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] mfd: rtsx: add func to split u32 into register
On Thu, Nov 27, 2014 at 10:53:58AM +0800, micky_ch...@realsil.com.cn wrote:
> +static inline void rtsx_pci_write_be32(struct rtsx_pcr *pcr, u16 reg, u32
> val)
> +{
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val >> 24);
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 16);
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 8);
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val);
This assumes the cpu is little endian. First convert to big endian
using cpu_to_be32() and then write it out.
__be32 be_val = cpu_to_be32()
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, be_val);
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, be_val >> 8);
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, be_val >> 16);
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, be_val >> 24);
(Written hurredly in my mail client. May be wrong).
> +}
> +
> +static inline void rtsx_pci_write_le32(struct rtsx_pcr *pcr, u16 reg, u32
> val)
> +{
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val);
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 8);
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 16);
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val >> 24);
> +}
We don't have a user for rtsx_pci_write_le32() so don't add it.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Thu, Nov 27, 2014 at 02:26:59PM +0100, Mariusz Gorski wrote: > On Wed, Nov 26, 2014 at 01:58:01PM -0800, Greg Kroah-Hartman wrote: > > On Wed, Nov 19, 2014 at 09:38:46PM +0100, Mariusz Gorski wrote: > > > Avoid magic number and use a comparison with a defined value instead > > > that checks whether module param has been set by the user to some > > > value at loading time. > > > > > > Signed-off-by: Mariusz Gorski > > > Acked-by: Willy Tarreau > > > --- > > > v2: Don't introduce new macros for param value check > > > > > > drivers/staging/panel/panel.c | 86 > > > +-- > > > 1 file changed, 43 insertions(+), 43 deletions(-) > > > > Ugh, I messed up here, and applied the first series, which was acked. > > > > Mariusz, can you resend the patches that I didn't apply, I can't seem to > > get the rest of these to work properly. > > Greg, if I get you here correctly, you've applied all 9 patches from v1 > and none from v2, so what you need now is a v1->v2 patch, right? No, I think I applied the patches sent _before_ the 9 series, it was 4 or 5 or so, you should have gotten an email about them. Pull my staging-testing branch and redo your remaining patches please. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:drivers:rtl8712:drv_types.h: Added blank line after declarations
This patch fixes the two checkpatch.pl warnings:
WARNING:Missing a blank line after declaration
Signed-off-by: Anjana Sasindran
---
drivers/staging/rtl8712/drv_types.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8712/drv_types.h
b/drivers/staging/rtl8712/drv_types.h
index 3d0a98b..3d8abfa 100644
--- a/drivers/staging/rtl8712/drv_types.h
+++ b/drivers/staging/rtl8712/drv_types.h
@@ -1,4 +1,4 @@
-/**
+0/**
*
* Copyright(c) 2007 - 2010 Realtek Corporation. All rights reserved.
*
@@ -129,6 +129,7 @@ struct dvobj_priv {
struct _adapter *padapter;
u32 nr_endpoint;
u8 ishighspeed;
+
uint(*inirp_init)(struct _adapter *adapter);
uint(*inirp_deinit)(struct _adapter *adapter);
struct usb_device *pusbdev;
@@ -166,6 +167,7 @@ struct _adapter {
pid_t evtThread;
struct task_struct *xmitThread;
pid_t recvThread;
+
uint(*dvobj_init)(struct _adapter *adapter);
void (*dvobj_deinit)(struct _adapter *adapter);
struct net_device *pnetdev;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] mmc: rtsx: add support for sdio card
> #ifdef DEBUG
> -static void sd_print_debug_regs(struct realtek_pci_sdmmc *host)
> +static void dump_reg_range(struct realtek_pci_sdmmc *host, u16 start, u16
> end)
> {
> - struct rtsx_pcr *pcr = host->pcr;
> - u16 i;
> - u8 *ptr;
> + u16 len = end - start + 1;
> + int i;
> + u8 *data = kzalloc(8, GFP_KERNEL);
The zeroing should be done inside the loop so that the last partial
read doesn't have old data. Just use an array on the stack here.
u8 data[8];
>
> - /* Print SD host internal registers */
> - rtsx_pci_init_cmd(pcr);
> - for (i = 0xFDA0; i <= 0xFDAE; i++)
> - rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
> - for (i = 0xFD52; i <= 0xFD69; i++)
> - rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
> - rtsx_pci_send_cmd(pcr, 100);
> -
> - ptr = rtsx_pci_get_cmd_data(pcr);
> - for (i = 0xFDA0; i <= 0xFDAE; i++)
> - dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
> - for (i = 0xFD52; i <= 0xFD69; i++)
> - dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
> + if (!data)
> + return;
> +
> + for (i = 0; i < len; i += 8, start += 8) {
I don't like this loop. Just leave start as is and write:
rtsx_pci_read_register(host->pcr, start + i + j,
data + j);
> + int j, n = min(8, len - i);
Put these declarations on separate lines.
The memset I mentioned earlier goes here.
memset(&data, 0, sizeof(data));
> +
> + for (j = 0; j < n; j++)
> + rtsx_pci_read_register(host->pcr, start + j, data + j);
> + dev_dbg(sdmmc_dev(host), "0x%04X(%d): %8ph\n", start, n, data);
> + }
> +
> + kfree(data);
> +}
> +
> +static void sd_print_debug_regs(struct realtek_pci_sdmmc *host)
> +{
> + dump_reg_range(host, 0xFDA0, 0xFDB3);
> + dump_reg_range(host, 0xFD52, 0xFD69);
> }
> #else
> #define sd_print_debug_regs(host)
> #endif /* DEBUG */
>
> +static int sdmmc_get_cd(struct mmc_host *mmc);
This forward declaration is not needed.
> +static void sd_send_cmd_get_rsp(struct realtek_pci_sdmmc *host,
> + struct mmc_command *cmd);
It's better to move the function forward, instead of having forward
declarations.
> +
> +static void sd_cmd_set_sd_cmd(struct rtsx_pcr *pcr, struct mmc_command *cmd)
> +{
> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_CMD0, 0xFF, 0x40 | cmd->opcode);
0x40 is a magic number.
> + rtsx_pci_write_be32(pcr, SD_CMD1, cmd->arg);
> +}
> +
[ snip ]
> @@ -293,47 +329,15 @@ static void sd_send_cmd_get_rsp(struct
> realtek_pci_sdmmc *host,
> int timeout = 100;
> int i;
> u8 *ptr;
> - int stat_idx = 0;
> - u8 rsp_type;
> - int rsp_len = 5;
> + int rsp_type = sd_response_type(cmd);
Don't do this assignment in the initializer. Put it next to the error
handling code. First we assign it. Then we use it. Then blank line.
Then we check it for errors. Spagghetttii.
> + int stat_idx = sd_status_index(rsp_type);
I have always hated this terrible pointer math. 5 is relative to
pcr->host_cmds_ptr + 1. It's a mess... :(
> bool clock_toggled = false;
[ snip ]
> -static int sd_rw_multi(struct realtek_pci_sdmmc *host, struct mmc_request
> *mrq)
> +static int sd_read_long_data(struct realtek_pci_sdmmc *host,
> + struct mmc_request *mrq)
> {
> struct rtsx_pcr *pcr = host->pcr;
> struct mmc_host *mmc = host->mmc;
> struct mmc_card *card = mmc->card;
> + struct mmc_command *cmd = mrq->cmd;
> struct mmc_data *data = mrq->data;
> int uhs = mmc_card_uhs(card);
> - int read = (data->flags & MMC_DATA_READ) ? 1 : 0;
> - u8 cfg2, trans_mode;
> + u8 cfg2 = 0;
> int err;
> + int resp_type = sd_response_type(cmd);
Same thing. Move this next to the error handling code.
[ snip ]
> @@ -653,14 +697,14 @@ static int sd_tuning_rx_cmd(struct realtek_pci_sdmmc
> *host,
> u8 opcode, u8 sample_point)
> {
> int err;
> - u8 cmd[5] = {0};
> + struct mmc_command cmd = {0};
I like the struct mmc_command changes but these seem like cleanups and
not needed for the new hardware. Send them as a separate patch instead
of mixed in with everything else.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:drivers:rtl8712:drv_types.h: Added blank line after declarations
On Thu, Nov 27, 2014 at 09:00:03PM +0530, Anjana Sasindran wrote: > This patch fixes the two checkpatch.pl warnings: > > WARNING:Missing a blank line after declaration > > Signed-off-by: Anjana Sasindran > --- > drivers/staging/rtl8712/drv_types.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8712/drv_types.h > b/drivers/staging/rtl8712/drv_types.h > index 3d0a98b..3d8abfa 100644 > --- a/drivers/staging/rtl8712/drv_types.h > +++ b/drivers/staging/rtl8712/drv_types.h > @@ -1,4 +1,4 @@ > -/** > +0/** > * > * Copyright(c) 2007 - 2010 Realtek Corporation. All rights reserved. > * You obviously did not compile this change :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Thu, Nov 27, 2014 at 07:24:17AM -0800, Greg Kroah-Hartman wrote: > On Thu, Nov 27, 2014 at 02:26:59PM +0100, Mariusz Gorski wrote: > > On Wed, Nov 26, 2014 at 01:58:01PM -0800, Greg Kroah-Hartman wrote: > > > On Wed, Nov 19, 2014 at 09:38:46PM +0100, Mariusz Gorski wrote: > > > > Avoid magic number and use a comparison with a defined value instead > > > > that checks whether module param has been set by the user to some > > > > value at loading time. > > > > > > > > Signed-off-by: Mariusz Gorski > > > > Acked-by: Willy Tarreau > > > > --- > > > > v2: Don't introduce new macros for param value check > > > > > > > > drivers/staging/panel/panel.c | 86 > > > > +-- > > > > 1 file changed, 43 insertions(+), 43 deletions(-) > > > > > > Ugh, I messed up here, and applied the first series, which was acked. > > > > > > Mariusz, can you resend the patches that I didn't apply, I can't seem to > > > get the rest of these to work properly. > > > > Greg, if I get you here correctly, you've applied all 9 patches from v1 > > and none from v2, so what you need now is a v1->v2 patch, right? > > No, I think I applied the patches sent _before_ the 9 series, it was 4 > or 5 or so, you should have gotten an email about them. Pull my > staging-testing branch and redo your remaining patches please. And the reason I got confused was because you didn't label your second set of patches "v2", which it was, I saw two separate series, one with a few patches, and then 2 sets of 9, the second set labeled "v2" so I thought they were independant. Please think of the poor maintainer who has to decipher things like this when you send them out... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: r8188eu: Add new device ID for DLink GO-USB-N150
The DLink GO-USB-N150 with revision B1 uses this driver.
Signed-off-by: Larry Finger
---
drivers/staging/rtl8188eu/os_dep/usb_intf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 65a257f..d3cbcc4 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -47,6 +47,7 @@ static struct usb_device_id rtw_usb_id_tbl[] = {
{USB_DEVICE(0x07b8, 0x8179)}, /* Abocom - Abocom */
{USB_DEVICE(0x2001, 0x330F)}, /* DLink DWA-125 REV D1 */
{USB_DEVICE(0x2001, 0x3310)}, /* Dlink DWA-123 REV D1 */
+ {USB_DEVICE(0x2001, 0x3311)}, /* DLink GO-USB-N150 REV B1 */
{USB_DEVICE(0x0df6, 0x0076)}, /* Sitecom N150 v2 */
{} /* Terminating entry */
};
--
2.1.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Thu, Nov 27, 2014 at 07:57:06AM -0800, Greg Kroah-Hartman wrote: > And the reason I got confused was because you didn't label your second > set of patches "v2", which it was, I saw two separate series, one with a > few patches, and then 2 sets of 9, the second set labeled "v2" so I > thought they were independant. Please think of the poor maintainer who > has to decipher things like this when you send them out... For Mariusz's defense, this was his first batch. He didn't feel comfortable and asked me how to proceed when sending a series and I forgot to warn him about the "v2" as initially I didn't think there would be a v2, and after I obviously forgot I didn't speak about that. So I share some responsibility for this one. Mariusz, if you need some help, tell me so. Regards, Willy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: octeon: Fix checkpatch warnings
Fixing 80 character limit warnings in octeon/ethernet-rx.c
Signed-off-by: Luis de Bethencourt
---
drivers/staging/octeon/ethernet-rx.c | 51 +---
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-rx.c
b/drivers/staging/octeon/ethernet-rx.c
index 1789a12..e387eb1 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -109,6 +109,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
int interface = cvmx_helper_get_interface_num(work->ipprt);
int index = cvmx_helper_get_interface_index_num(work->ipprt);
union cvmx_gmxx_rxx_frm_ctl gmxx_rxx_frm_ctl;
+
gmxx_rxx_frm_ctl.u64 =
cvmx_read_csr(CVMX_GMXX_RXX_FRM_CTL(index, interface));
if (gmxx_rxx_frm_ctl.s.pre_chk == 0) {
@@ -126,13 +127,15 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t
*work)
if (*ptr == 0xd5) {
/*
- printk_ratelimited("Port %d received 0xd5
preamble\n", work->ipprt);
+ printk_ratelimited("Port %d received 0xd5
preamble\n",
+ work->ipprt);
*/
work->packet_ptr.s.addr += i + 1;
work->len -= i + 5;
} else if ((*ptr & 0xf) == 0xd) {
/*
- printk_ratelimited("Port %d received 0x?d
preamble\n", work->ipprt);
+ printk_ratelimited("Port %d received 0x?d
preamble\n",
+ work->ipprt);
*/
work->packet_ptr.s.addr += i;
work->len -= i + 4;
@@ -212,17 +215,20 @@ static int cvm_oct_napi_poll(struct napi_struct *napi,
int budget)
did_work_request = 0;
if (work == NULL) {
union cvmx_pow_wq_int wq_int;
+
wq_int.u64 = 0;
wq_int.s.iq_dis = 1 << pow_receive_group;
wq_int.s.wq_int = 1 << pow_receive_group;
cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
break;
}
- pskb = (struct sk_buff
**)(cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *));
+ pskb = (struct sk_buff
**)(cvm_oct_get_buffer_ptr(work->packet_ptr) -
+ sizeof(void *));
prefetch(pskb);
if (USE_ASYNC_IOBDMA && rx_count < (budget - 1)) {
- cvmx_pow_work_request_async_nocheck(CVMX_SCR_SCRATCH,
CVMX_POW_NO_WAIT);
+ cvmx_pow_work_request_async_nocheck(CVMX_SCR_SCRATCH,
+ CVMX_POW_NO_WAIT);
did_work_request = 1;
}
rx_count++;
@@ -247,7 +253,8 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int
budget)
* buffer.
*/
if (likely(skb_in_hw)) {
- skb->data = skb->head + work->packet_ptr.s.addr -
cvmx_ptr_to_phys(skb->head);
+ skb->data = skb->head + work->packet_ptr.s.addr -
+ cvmx_ptr_to_phys(skb->head);
prefetch(skb->data);
skb->len = work->len;
skb_set_tail_pointer(skb, skb->len);
@@ -284,7 +291,8 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int
budget)
/* No packet buffers to free */
} else {
int segments = work->word2.s.bufs;
- union cvmx_buf_ptr segment_ptr =
work->packet_ptr;
+ union cvmx_buf_ptr segment_ptr =
+ work->packet_ptr;
int len = work->len;
while (segments--) {
@@ -300,8 +308,11 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int
budget)
* one: int segment_size =
* segment_ptr.s.size;
*/
- int segment_size =
CVMX_FPA_PACKET_POOL_SIZE -
- (segment_ptr.s.addr -
(((segment_ptr.s.addr >> 7) - segment_ptr.s.back) << 7));
+ int segment_size =
+ CVMX_FPA_PACKET_POOL_SIZE -
+ (segment_ptr.s.addr -
+ (((segment_ptr.s.addr >> 7) -
+
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Thu, Nov 27, 2014 at 05:14:06PM +0100, Willy Tarreau wrote: > On Thu, Nov 27, 2014 at 07:57:06AM -0800, Greg Kroah-Hartman wrote: > > And the reason I got confused was because you didn't label your second > > set of patches "v2", which it was, I saw two separate series, one with a > > few patches, and then 2 sets of 9, the second set labeled "v2" so I > > thought they were independant. Please think of the poor maintainer who > > has to decipher things like this when you send them out... > > For Mariusz's defense, this was his first batch. He didn't feel comfortable > and asked me how to proceed when sending a series and I forgot to warn him > about the "v2" as initially I didn't think there would be a v2, and after I > obviously forgot I didn't speak about that. So I share some responsibility > for this one. I wasn't trying to make anyone feel bad here, just trying to explain why I got this all messed up on my end. Mariusz, consider this a chance to learn how to rebase your patches, a very common task for kernel developers to have to do :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: r8188eu: Add new device ID for DLink GO-USB-N150
On Thu, Nov 27, 2014 at 10:10:21AM -0600, Larry Finger wrote:
> The DLink GO-USB-N150 with revision B1 uses this driver.
>
> Signed-off-by: Larry Finger
> ---
> drivers/staging/rtl8188eu/os_dep/usb_intf.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> index 65a257f..d3cbcc4 100644
> --- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> @@ -47,6 +47,7 @@ static struct usb_device_id rtw_usb_id_tbl[] = {
> {USB_DEVICE(0x07b8, 0x8179)}, /* Abocom - Abocom */
> {USB_DEVICE(0x2001, 0x330F)}, /* DLink DWA-125 REV D1 */
> {USB_DEVICE(0x2001, 0x3310)}, /* Dlink DWA-123 REV D1 */
> + {USB_DEVICE(0x2001, 0x3311)}, /* DLink GO-USB-N150 REV B1 */
> {USB_DEVICE(0x0df6, 0x0076)}, /* Sitecom N150 v2 */
> {} /* Terminating entry */
> };
> --
> 2.1.2
Should this also go to stable kernels?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: octeon: Fix checkpatch warnings
On Thu, 2014-11-27 at 17:18 +0100, Luis de Bethencourt wrote:
> Fixing 80 character limit warnings in octeon/ethernet-rx.c
Hello again Luis.
Another thing you might consider is to align
multiple line statements to the appropriate
open parenthesis.
> diff --git a/drivers/staging/octeon/ethernet-rx.c
> b/drivers/staging/octeon/ethernet-rx.c
[]
> @@ -126,13 +127,15 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t
> *work)
>
> if (*ptr == 0xd5) {
> /*
> - printk_ratelimited("Port %d received 0xd5
> preamble\n", work->ipprt);
> + printk_ratelimited("Port %d received 0xd5
> preamble\n",
> + work->ipprt);
>*/
This is in a commented out block, but this
would look better like:
printk_ratelimited("format",
args...);
> @@ -212,17 +215,20 @@ static int cvm_oct_napi_poll(struct napi_struct *napi,
> int budget)
> did_work_request = 0;
> if (work == NULL) {
> union cvmx_pow_wq_int wq_int;
> +
> wq_int.u64 = 0;
> wq_int.s.iq_dis = 1 << pow_receive_group;
> wq_int.s.wq_int = 1 << pow_receive_group;
> cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
> break;
> }
> - pskb = (struct sk_buff
> **)(cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *));
> + pskb = (struct sk_buff
> **)(cvm_oct_get_buffer_ptr(work->packet_ptr) -
> + sizeof(void *));
cvm_oct_get_buffer_ptr returns a void pointer
so it doesn't need a cast.
a possible fix is just to remove the cast
pskb = cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void
*);
> prefetch(pskb);
If the cast type above to a ** is to be believed,
maybe this should be prefetch(*pskb)
If you do these, they should be separate patches.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: r8188eu: Add new device ID for DLink GO-USB-N150
On 11/27/2014 10:28 AM, Greg KH wrote:
On Thu, Nov 27, 2014 at 10:10:21AM -0600, Larry Finger wrote:
The DLink GO-USB-N150 with revision B1 uses this driver.
Signed-off-by: Larry Finger
---
drivers/staging/rtl8188eu/os_dep/usb_intf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 65a257f..d3cbcc4 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -47,6 +47,7 @@ static struct usb_device_id rtw_usb_id_tbl[] = {
{USB_DEVICE(0x07b8, 0x8179)}, /* Abocom - Abocom */
{USB_DEVICE(0x2001, 0x330F)}, /* DLink DWA-125 REV D1 */
{USB_DEVICE(0x2001, 0x3310)}, /* Dlink DWA-123 REV D1 */
+ {USB_DEVICE(0x2001, 0x3311)}, /* DLink GO-USB-N150 REV B1 */
{USB_DEVICE(0x0df6, 0x0076)}, /* Sitecom N150 v2 */
{} /* Terminating entry */
};
--
2.1.2
Should this also go to stable kernels?
Yes, it should. Sorry for missing that Cc.
Larry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: octeon: Fix checkpatch warnings
On Thu, Nov 27, 2014 at 08:34:00AM -0800, Joe Perches wrote:
> On Thu, 2014-11-27 at 17:18 +0100, Luis de Bethencourt wrote:
> > Fixing 80 character limit warnings in octeon/ethernet-rx.c
>
> Hello again Luis.
>
> Another thing you might consider is to align
> multiple line statements to the appropriate
> open parenthesis.
Sure :) I will do this and merge it into the patch in a few hours.
>
> > diff --git a/drivers/staging/octeon/ethernet-rx.c
> > b/drivers/staging/octeon/ethernet-rx.c
> []
> > @@ -126,13 +127,15 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t
> > *work)
> >
> > if (*ptr == 0xd5) {
> > /*
> > - printk_ratelimited("Port %d received 0xd5
> > preamble\n", work->ipprt);
> > + printk_ratelimited("Port %d received 0xd5
> > preamble\n",
> > + work->ipprt);
> > */
>
> This is in a commented out block, but this
> would look better like:
>
> printk_ratelimited("format",
> args...);
>
I agree. Thanks for the pointer.
>
> > @@ -212,17 +215,20 @@ static int cvm_oct_napi_poll(struct napi_struct
> > *napi, int budget)
> > did_work_request = 0;
> > if (work == NULL) {
> > union cvmx_pow_wq_int wq_int;
> > +
> > wq_int.u64 = 0;
> > wq_int.s.iq_dis = 1 << pow_receive_group;
> > wq_int.s.wq_int = 1 << pow_receive_group;
> > cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
> > break;
> > }
> > - pskb = (struct sk_buff
> > **)(cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void *));
> > + pskb = (struct sk_buff
> > **)(cvm_oct_get_buffer_ptr(work->packet_ptr) -
> > + sizeof(void *));
>
> cvm_oct_get_buffer_ptr returns a void pointer
> so it doesn't need a cast.
>
> a possible fix is just to remove the cast
>
> pskb = cvm_oct_get_buffer_ptr(work->packet_ptr) - sizeof(void
> *);
>
> > prefetch(pskb);
>
> If the cast type above to a ** is to be believed,
> maybe this should be prefetch(*pskb)
>
> If you do these, they should be separate patches.
>
Cool! Once I do the above I will fix this in a separate patch.
Thanks again for your time reviewing Joe,
Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: fix sparse warnings related to lock context imbalance
Hello Greg,
After some investigation, I think that removing these wrappers is not going to
improve the code readability:
On Wed, Nov 26, 2014 at 12:54:43PM -0800, Greg KH wrote:
> On Wed, Nov 26, 2014 at 05:15:48PM +0100, Loic Pefferkorn wrote:
> > Add __acquires() and __releases() function annotations, to fix sparse
> > warnings related to lock context imbalance.
> >
> > This fixes the following warnings:
> >
> > drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c:153:5:
> > warning: context imbalance in 'cfs_trace_lock_tcd' - wrong count at exit
> > drivers/staging/lustre/lustre/libcfs/hash.c:128:1: warning: context
> > imbalance in 'cfs_hash_spin_lock' - wrong count at exit
> > drivers/staging/lustre/lustre/libcfs/hash.c:142:9: warning: context
> > imbalance in 'cfs_hash_rw_lock' - wrong count at exit
> > drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:57:17:
> > warning: context imbalance in 'lock_res_and_lock' - wrong count at exit
> > drivers/staging/lustre/lustre/libcfs/libcfs_lock.c:93:1: warning: context
> > imbalance in 'cfs_percpt_lock' - wrong count at exit
> >
> > Signed-off-by: Loic Pefferkorn
> > ---
> > drivers/staging/lustre/lustre/libcfs/hash.c | 4
> > drivers/staging/lustre/lustre/libcfs/libcfs_lock.c | 2 ++
> > drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c | 2 ++
> > drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 ++
> > 4 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c
> > b/drivers/staging/lustre/lustre/libcfs/hash.c
> > index 32da783..7c6e2a3 100644
> > --- a/drivers/staging/lustre/lustre/libcfs/hash.c
> > +++ b/drivers/staging/lustre/lustre/libcfs/hash.c
> > @@ -126,18 +126,21 @@ cfs_hash_nl_unlock(union cfs_hash_lock *lock, int
> > exclusive) {}
> >
> > static inline void
> > cfs_hash_spin_lock(union cfs_hash_lock *lock, int exclusive)
> > + __acquires(&lock->spin)
> > {
> > spin_lock(&lock->spin);
> > }
> >
> > static inline void
> > cfs_hash_spin_unlock(union cfs_hash_lock *lock, int exclusive)
> > + __releases(&lock->spin)
> > {
> > spin_unlock(&lock->spin);
> > }
>
> Ugh, how horrid, please just delete these functions and push down the
> spin_lock/unlock calls down into the places these are called.
cfs_hash_spin_lock() and cfs_hash_spin_unlock() are referenced by function
pointers later in the same file:
165 /** no bucket lock, one spinlock to protect everything */
166 static cfs_hash_lock_ops_t cfs_hash_nbl_lops = {
167 .hs_lock= cfs_hash_spin_lock,
168 .hs_unlock = cfs_hash_spin_unlock,
169 .hs_bkt_lock= cfs_hash_nl_lock,
170 .hs_bkt_unlock = cfs_hash_nl_unlock,
171 };
172
173 /** spin bucket lock, rehash is enabled */
174 static cfs_hash_lock_ops_t cfs_hash_bkt_spin_lops = {
175 .hs_lock= cfs_hash_rw_lock,
176 .hs_unlock = cfs_hash_rw_unlock,
177 .hs_bkt_lock= cfs_hash_spin_lock,
178 .hs_bkt_unlock = cfs_hash_spin_unlock,
179 };
>
> >
> > static inline void
> > cfs_hash_rw_lock(union cfs_hash_lock *lock, int exclusive)
> > + __acquires(&lock->rw)
> > {
> > if (!exclusive)
> > read_lock(&lock->rw);
> > @@ -147,6 +150,7 @@ cfs_hash_rw_lock(union cfs_hash_lock *lock, int
> > exclusive)
> >
> > static inline void
> > cfs_hash_rw_unlock(union cfs_hash_lock *lock, int exclusive)
> > + __releases(&lock->rw)
> > {
> > if (!exclusive)
> > read_unlock(&lock->rw);
>
>
> Same for these.
Likewise for cfs_hash_rw_lock() and cfs_hash_rw_unlock():
173 /** spin bucket lock, rehash is enabled */
174 static cfs_hash_lock_ops_t cfs_hash_bkt_spin_lops = {
175 .hs_lock= cfs_hash_rw_lock,
176 .hs_unlock = cfs_hash_rw_unlock,
177 .hs_bkt_lock= cfs_hash_spin_lock,
178 .hs_bkt_unlock = cfs_hash_spin_unlock,
179 };
180
181 /** rw bucket lock, rehash is enabled */
182 static cfs_hash_lock_ops_t cfs_hash_bkt_rw_lops = {
183 .hs_lock= cfs_hash_rw_lock,
184 .hs_unlock = cfs_hash_rw_unlock,
185 .hs_bkt_lock= cfs_hash_rw_lock,
186 .hs_bkt_unlock = cfs_hash_rw_unlock,
187 };
>
> > diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_lock.c
> > b/drivers/staging/lustre/lustre/libcfs/libcfs_lock.c
> > index 2c199c7..1e529fc 100644
> > --- a/drivers/staging/lustre/lustre/libcfs/libcfs_lock.c
> > +++ b/drivers/staging/lustre/lustre/libcfs/libcfs_lock.c
> > @@ -91,6 +91,7 @@ EXPORT_SYMBOL(cfs_percpt_lock_alloc);
> > */
> > void
> > cfs_percpt_lock(struct cfs_percpt_lock *pcl, int index)
> > + __acquires(pcl->pcl_locks[index])
> > {
> > int ncpt = cfs_cpt_number(pcl->pcl_cptab);
> > int i;
> > @@ -125,6 +126,7 @@ EXPORT_SYMBOL(cfs_percpt_lock);
> > /** unlock a CPU partition */
> > void
> > cfs_percpt_unlock(struct cfs_percpt_lock *pcl, int index)
> > + __releases(pcl->pcl_lock
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Thu, Nov 27, 2014 at 07:57:06AM -0800, Greg Kroah-Hartman wrote: > On Thu, Nov 27, 2014 at 07:24:17AM -0800, Greg Kroah-Hartman wrote: > > On Thu, Nov 27, 2014 at 02:26:59PM +0100, Mariusz Gorski wrote: > > > On Wed, Nov 26, 2014 at 01:58:01PM -0800, Greg Kroah-Hartman wrote: > > > > On Wed, Nov 19, 2014 at 09:38:46PM +0100, Mariusz Gorski wrote: > > > > > Avoid magic number and use a comparison with a defined value instead > > > > > that checks whether module param has been set by the user to some > > > > > value at loading time. > > > > > > > > > > Signed-off-by: Mariusz Gorski > > > > > Acked-by: Willy Tarreau > > > > > --- > > > > > v2: Don't introduce new macros for param value check > > > > > > > > > > drivers/staging/panel/panel.c | 86 > > > > > +-- > > > > > 1 file changed, 43 insertions(+), 43 deletions(-) > > > > > > > > Ugh, I messed up here, and applied the first series, which was acked. > > > > > > > > Mariusz, can you resend the patches that I didn't apply, I can't seem to > > > > get the rest of these to work properly. > > > > > > Greg, if I get you here correctly, you've applied all 9 patches from v1 > > > and none from v2, so what you need now is a v1->v2 patch, right? > > > > No, I think I applied the patches sent _before_ the 9 series, it was 4 > > or 5 or so, you should have gotten an email about them. Pull my > > staging-testing branch and redo your remaining patches please. > > And the reason I got confused was because you didn't label your second > set of patches "v2", which it was, I saw two separate series, one with a > few patches, and then 2 sets of 9, the second set labeled "v2" so I > thought they were independant. Please think of the poor maintainer who > has to decipher things like this when you send them out... I'm confused right now. As you say, first I've sent a patchset of 4: https://lkml.org/lkml/2014/11/11/963 Then, a couple of days later, I've sent the initial patchset of 9: https://lkml.org/lkml/2014/11/18/922 And a day I've sent a fixed version of the above patchset, labeled with v2: https://lkml.org/lkml/2014/11/19/653 Isn't this the right way to do? I still don't get my mistake. Because what I was just about to do is to resend the v2 patchset, but now I'm not sure anymore if this is what I'm supposed to do. BTW: Out of these 3 patchsets, 1st and 3rd should be applied. > thanks, > > greg k-h Cheers, Mariusz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Thu, Nov 27, 2014 at 08:50:55PM +0100, Mariusz Gorski wrote: > > And the reason I got confused was because you didn't label your second > > set of patches "v2", which it was, I saw two separate series, one with a > > few patches, and then 2 sets of 9, the second set labeled "v2" so I > > thought they were independant. Please think of the poor maintainer who > > has to decipher things like this when you send them out... > > I'm confused right now. As you say, first I've sent a patchset of 4: > https://lkml.org/lkml/2014/11/11/963 > > Then, a couple of days later, I've sent the initial patchset of 9: > https://lkml.org/lkml/2014/11/18/922 > > And a day I've sent a fixed version of the above patchset, labeled with v2: > https://lkml.org/lkml/2014/11/19/653 > > Isn't this the right way to do? I still don't get my mistake. Because > what I was just about to do is to resend the v2 patchset, but now I'm > not sure anymore if this is what I'm supposed to do. > > BTW: Out of these 3 patchsets, 1st and 3rd should be applied. Mariusz, for people who have to parse hundreds to thousands of e-mails a day, dealing with non-trivial operation modes like this is never easy. I think (I'll let Greg suggest what he prefers) that the most reliable thing to do *right now* is to rebase your tree on top of Greg's staging tree, and you send the resulting series (what you apply *after* staging) at once, maybe even tagged as v3 to avoid any confusion. Sometimes for the recipient, things apparently as simple as sorting e-mails by subjects to find something can cause some confusion when it's not obvious what replaces what, and tagging with the version or ensuring that each series is different enough helps avoiding this. If you need any help, contact me off-list and I'll gladly help you. Don't worry, issues like this commonly happen and will happen again, whatever you do against them will only reduce the likeliness that they happen again (and that's important to care about this) :-) Thanks, Willy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state
On Thu, Nov 27, 2014 at 7:05 PM, Willy Tarreau wrote: > Mariusz, for people who have to parse hundreds to thousands of e-mails > a day, dealing with non-trivial operation modes like this is never easy. > > I think (I'll let Greg suggest what he prefers) that the most reliable > thing to do *right now* is to rebase your tree on top of Greg's staging > tree, and you send the resulting series (what you apply *after* staging) Actually 'staging-testing' branch would be better: https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/log/?h=staging-testing&ofs=100 There are 4 patches from Mariusz already applied there. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/9] staging: panel: Call init function directly
Remove useless function and let the kernel call the actual
init function directly.
Signed-off-by: Mariusz Gorski
Acked-by: Willy Tarreau
---
drivers/staging/panel/panel.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 3dd318a..0d3f09e 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -,7 +,7 @@ static struct parport_driver panel_driver = {
};
/* init function */
-static int panel_init(void)
+static int __init panel_init_module(void)
{
/* for backwards compatibility */
if (keypad_type < 0)
@@ -2334,11 +2334,6 @@ static int panel_init(void)
return 0;
}
-static int __init panel_init_module(void)
-{
- return panel_init();
-}
-
static void __exit panel_cleanup_module(void)
{
unregister_reboot_notifier(&panel_notifier);
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 8/9] staging: panel: Remove more magic number comparison
Use a defined value instead of magic number comparison for checking whether a module param value has been set. Signed-off-by: Mariusz Gorski Acked-by: Willy Tarreau --- drivers/staging/panel/panel.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c index 0bdb447..19f6767 100644 --- a/drivers/staging/panel/panel.c +++ b/drivers/staging/panel/panel.c @@ -1494,17 +1494,17 @@ static void lcd_init(void) } /* Overwrite with module params set on loading */ - if (lcd_height > -1) + if (lcd_height != NOT_SET) lcd.height = lcd_height; - if (lcd_width > -1) + if (lcd_width != NOT_SET) lcd.width = lcd_width; - if (lcd_bwidth > -1) + if (lcd_bwidth != NOT_SET) lcd.bwidth = lcd_bwidth; - if (lcd_hwidth > -1) + if (lcd_hwidth != NOT_SET) lcd.hwidth = lcd_hwidth; - if (lcd_charset > -1) + if (lcd_charset != NOT_SET) lcd.charset = lcd_charset; - if (lcd_proto > -1) + if (lcd_proto != NOT_SET) lcd.proto = lcd_proto; if (lcd_e_pin != PIN_NOT_SET) lcd.pins.e = lcd_e_pin; @@ -2304,16 +2304,16 @@ static int __init panel_init_module(void) * Overwrite selection with module param values (both keypad and lcd), * where the deprecated params have lower prio. */ - if (keypad_enabled > -1) + if (keypad_enabled != NOT_SET) selected_keypad_type = keypad_enabled; - if (keypad_type > -1) + if (keypad_type != NOT_SET) selected_keypad_type = keypad_type; keypad.enabled = (selected_keypad_type > 0); - if (lcd_enabled > -1) + if (lcd_enabled != NOT_SET) selected_lcd_type = lcd_enabled; - if (lcd_type > -1) + if (lcd_type != NOT_SET) selected_lcd_type = lcd_type; lcd.enabled = (selected_lcd_type > 0); -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/9] staging: panel: Refactor panel initialization
This set of patches focuses on making current initialization process easier to understand - especially it tries to emphasize what are the priorities of the params coming from different sources (Kconfig values, device profiles and module param values set on loading). I paid attention not to change to behaviour of the code itself (at least for now), so all hacky places are kept. The whole patchset is already: Acked-by: Willy Tarreau v2: Don't introduce new macros for param value check v3: Resend, no other changes Mariusz Gorski (9): staging: panel: Set default parport module param value staging: panel: Call init function directly staging: panel: Remove magic numbers staging: panel: Use defined value or checking module params state staging: panel: Start making module params read-only staging: panel: Make two more module params read-only staging: panel: Refactor LCD init code staging: panel: Remove more magic number comparison staging: panel: Move LCD-related state into struct lcd drivers/staging/panel/panel.c | 669 ++ 1 file changed, 354 insertions(+), 315 deletions(-) -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/9] staging: panel: Remove magic numbers
Get rid of magic numbers indicating that the value of a module param is not set. Use a defined value instead. Signed-off-by: Mariusz Gorski Acked-by: Willy Tarreau --- drivers/staging/panel/panel.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c index 0d3f09e..1b4a211 100644 --- a/drivers/staging/panel/panel.c +++ b/drivers/staging/panel/panel.c @@ -133,6 +133,8 @@ #define LCD_ESCAPE_LEN 24 /* max chars for LCD escape command */ #define LCD_ESCAPE_CHAR27 /* use char 27 for escape command */ +#define NOT_SET-1 + /* macros to simplify use of the parallel port */ #define r_ctr(x)(parport_read_control((x)->port)) #define r_dtr(x)(parport_read_data((x)->port)) @@ -439,37 +441,37 @@ MODULE_PARM_DESC(profile, "1=16x2 old kp; 2=serial 16x2, new kp; 3=16x2 hantronix; " "4=16x2 nexcom; default=40x2, old kp"); -static int keypad_type = -1; +static int keypad_type = NOT_SET; module_param(keypad_type, int, ); MODULE_PARM_DESC(keypad_type, "Keypad type: 0=none, 1=old 6 keys, 2=new 6+1 keys, 3=nexcom 4 keys"); -static int lcd_type = -1; +static int lcd_type = NOT_SET; module_param(lcd_type, int, ); MODULE_PARM_DESC(lcd_type, "LCD type: 0=none, 1=old //, 2=serial ks0074, 3=hantronix //, 4=nexcom //, 5=compiled-in"); -static int lcd_height = -1; +static int lcd_height = NOT_SET; module_param(lcd_height, int, ); MODULE_PARM_DESC(lcd_height, "Number of lines on the LCD"); -static int lcd_width = -1; +static int lcd_width = NOT_SET; module_param(lcd_width, int, ); MODULE_PARM_DESC(lcd_width, "Number of columns on the LCD"); -static int lcd_bwidth = -1;/* internal buffer width (usually 40) */ +static int lcd_bwidth = NOT_SET; /* internal buffer width (usually 40) */ module_param(lcd_bwidth, int, ); MODULE_PARM_DESC(lcd_bwidth, "Internal LCD line width (40)"); -static int lcd_hwidth = -1;/* hardware buffer width (usually 64) */ +static int lcd_hwidth = NOT_SET; /* hardware buffer width (usually 64) */ module_param(lcd_hwidth, int, ); MODULE_PARM_DESC(lcd_hwidth, "LCD line hardware address (64)"); -static int lcd_charset = -1; +static int lcd_charset = NOT_SET; module_param(lcd_charset, int, ); MODULE_PARM_DESC(lcd_charset, "LCD character set: 0=standard, 1=KS0074"); -static int lcd_proto = -1; +static int lcd_proto = NOT_SET; module_param(lcd_proto, int, ); MODULE_PARM_DESC(lcd_proto, "LCD communication: 0=parallel (//), 1=serial, 2=TI LCD Interface"); @@ -515,11 +517,11 @@ MODULE_PARM_DESC(lcd_bl_pin, /* Deprecated module parameters - consider not using them anymore */ -static int lcd_enabled = -1; +static int lcd_enabled = NOT_SET; module_param(lcd_enabled, int, ); MODULE_PARM_DESC(lcd_enabled, "Deprecated option, use lcd_type instead"); -static int keypad_enabled = -1; +static int keypad_enabled = NOT_SET; module_param(keypad_enabled, int, ); MODULE_PARM_DESC(keypad_enabled, "Deprecated option, use keypad_type instead"); -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/9] staging: panel: Use defined value or checking module params state
Avoid magic number and use a comparison with a defined value instead
that checks whether module param has been set by the user to some
value at loading time.
Signed-off-by: Mariusz Gorski
Acked-by: Willy Tarreau
---
drivers/staging/panel/panel.c | 86 +--
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 1b4a211..5288990 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1411,29 +1411,29 @@ static void lcd_init(void)
switch (lcd_type) {
case LCD_TYPE_OLD:
/* parallel mode, 8 bits */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_STROBE;
if (lcd_rs_pin == PIN_NOT_SET)
lcd_rs_pin = PIN_AUTOLF;
- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 40;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 64;
- if (lcd_height < 0)
+ if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_KS0074:
/* serial mode, ks0074 */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_SERIAL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_KS0074;
if (lcd_bl_pin == PIN_NOT_SET)
lcd_bl_pin = PIN_AUTOLF;
@@ -1442,20 +1442,20 @@ static void lcd_init(void)
if (lcd_da_pin == PIN_NOT_SET)
lcd_da_pin = PIN_D0;
- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 16;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 16;
- if (lcd_height < 0)
+ if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_NEXCOM:
/* parallel mode, 8 bits, generic */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_AUTOLF;
@@ -1464,42 +1464,42 @@ static void lcd_init(void)
if (lcd_rw_pin == PIN_NOT_SET)
lcd_rw_pin = PIN_INITP;
- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 16;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
- if (lcd_hwidth < 0)
+ if (lcd_hwidth == NOT_SET)
lcd_hwidth = 64;
- if (lcd_height < 0)
+ if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_CUSTOM:
/* customer-defined */
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = DEFAULT_LCD_PROTO;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = DEFAULT_LCD_CHARSET;
/* default geometry will be set later */
break;
case LCD_TYPE_HANTRONIX:
/* parallel mode, 8 bits, hantronix-like */
default:
- if (lcd_proto < 0)
+ if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
- if (lcd_charset < 0)
+ if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_STROBE;
if (lcd_rs_pin == PIN_NOT_SET)
lcd_rs_pin = PIN_SELECP;
- if (lcd_width < 0)
+ if (lcd_width == NOT_SET)
lcd_width = 16;
- if (lcd_bwidth < 0)
+ if (lcd_bwidth =
[PATCH v3 5/9] staging: panel: Start making module params read-only
Start decoupling module params from the actual device state,
both for lcd and keypad, by keeping the params read-only
and moving the device state to related structs.
Signed-off-by: Mariusz Gorski
Acked-by: Willy Tarreau
---
drivers/staging/panel/panel.c | 35 +--
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 5288990..7e5bb53 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -212,6 +212,10 @@ static pmask_t phys_prev;
static char inputs_stable;
/* these variables are specific to the keypad */
+static struct {
+ bool enabled;
+} keypad;
+
static char keypad_buffer[KEYPAD_BUFFER];
static int keypad_buflen;
static int keypad_start;
@@ -219,6 +223,9 @@ static char keypressed;
static wait_queue_head_t keypad_read_wait;
/* lcd-specific variables */
+static struct {
+ bool enabled;
+} lcd;
/* contains the LCD config state */
static unsigned long int lcd_flags;
@@ -1393,7 +1400,7 @@ static void panel_lcd_print(const char *s)
const char *tmp = s;
int count = strlen(s);
- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
for (; count-- > 0; tmp++) {
if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
/* let's be a little nice with other processes
@@ -1923,7 +1930,7 @@ static void panel_process_inputs(void)
static void panel_scan_timer(void)
{
- if (keypad_enabled && keypad_initialized) {
+ if (keypad.enabled && keypad_initialized) {
if (spin_trylock_irq(&pprt_lock)) {
phys_scan_contacts();
@@ -1935,7 +1942,7 @@ static void panel_scan_timer(void)
panel_process_inputs();
}
- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
if (keypressed) {
if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
lcd_backlight(1);
@@ -2114,7 +2121,7 @@ static void keypad_init(void)
static int panel_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
{
- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
switch (code) {
case SYS_DOWN:
panel_lcd_print
@@ -2170,13 +2177,13 @@ static void panel_attach(struct parport *port)
/* must init LCD first, just in case an IRQ from the keypad is
* generated at keypad init
*/
- if (lcd_enabled) {
+ if (lcd.enabled) {
lcd_init();
if (misc_register(&lcd_dev))
goto err_unreg_device;
}
- if (keypad_enabled) {
+ if (keypad.enabled) {
keypad_init();
if (misc_register(&keypad_dev))
goto err_lcd_unreg;
@@ -2184,7 +2191,7 @@ static void panel_attach(struct parport *port)
return;
err_lcd_unreg:
- if (lcd_enabled)
+ if (lcd.enabled)
misc_deregister(&lcd_dev);
err_unreg_device:
parport_unregister_device(pprt);
@@ -2202,12 +2209,12 @@ static void panel_detach(struct parport *port)
return;
}
- if (keypad_enabled && keypad_initialized) {
+ if (keypad.enabled && keypad_initialized) {
misc_deregister(&keypad_dev);
keypad_initialized = 0;
}
- if (lcd_enabled && lcd_initialized) {
+ if (lcd.enabled && lcd_initialized) {
misc_deregister(&lcd_dev);
lcd_initialized = 0;
}
@@ -2283,8 +2290,8 @@ static int __init panel_init_module(void)
break;
}
- lcd_enabled = (lcd_type > 0);
- keypad_enabled = (keypad_type > 0);
+ lcd.enabled = (lcd_type > 0);
+ keypad.enabled = (keypad_type > 0);
switch (keypad_type) {
case KEYPAD_TYPE_OLD:
@@ -2309,7 +2316,7 @@ static int __init panel_init_module(void)
return -EIO;
}
- if (!lcd_enabled && !keypad_enabled) {
+ if (!lcd.enabled && !keypad.enabled) {
/* no device enabled, let's release the parport */
if (pprt) {
parport_release(pprt);
@@ -2344,12 +2351,12 @@ static void __exit panel_cleanup_module(void)
del_timer_sync(&scan_timer);
if (pprt != NULL) {
- if (keypad_enabled) {
+ if (keypad.enabled) {
misc_deregister(&keypad_dev);
keypad_initialized = 0;
}
- if (lcd_enabled) {
+ if (lcd.enabled) {
panel_lcd_print("\x0cLCD driver " PANEL_VERSION
[PATCH v3 6/9] staging: panel: Make two more module params read-only
Make keypad_type and lcd_type module params read-only.
This step also starts making it more clear what is
the precedence of device params coming from different
sources (device profile, runtime module param values etc).
Signed-off-by: Mariusz Gorski
Acked-by: Willy Tarreau
---
drivers/staging/panel/panel.c | 71 ++-
1 file changed, 37 insertions(+), 34 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 7e5bb53..5868a28 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -227,6 +227,9 @@ static struct {
bool enabled;
} lcd;
+/* Needed only for init */
+static int selected_lcd_type = NOT_SET;
+
/* contains the LCD config state */
static unsigned long int lcd_flags;
/* contains the LCD X offset */
@@ -1415,7 +1418,7 @@ static void panel_lcd_print(const char *s)
/* initialize the LCD driver */
static void lcd_init(void)
{
- switch (lcd_type) {
+ switch (selected_lcd_type) {
case LCD_TYPE_OLD:
/* parallel mode, 8 bits */
if (lcd_proto == NOT_SET)
@@ -2233,28 +2236,21 @@ static struct parport_driver panel_driver = {
/* init function */
static int __init panel_init_module(void)
{
- /* for backwards compatibility */
- if (keypad_type == NOT_SET)
- keypad_type = keypad_enabled;
-
- if (lcd_type == NOT_SET)
- lcd_type = lcd_enabled;
+ int selected_keypad_type = NOT_SET;
/* take care of an eventual profile */
switch (profile) {
case PANEL_PROFILE_CUSTOM:
/* custom profile */
- if (keypad_type == NOT_SET)
- keypad_type = DEFAULT_KEYPAD_TYPE;
- if (lcd_type == NOT_SET)
- lcd_type = DEFAULT_LCD_TYPE;
+ selected_keypad_type = DEFAULT_KEYPAD_TYPE;
+ selected_lcd_type = DEFAULT_LCD_TYPE;
break;
case PANEL_PROFILE_OLD:
/* 8 bits, 2*16, old keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_OLD;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_OLD;
+ selected_keypad_type = KEYPAD_TYPE_OLD;
+ selected_lcd_type = LCD_TYPE_OLD;
+
+ /* TODO: This two are a little hacky, sort it out later */
if (lcd_width == NOT_SET)
lcd_width = 16;
if (lcd_hwidth == NOT_SET)
@@ -2262,38 +2258,45 @@ static int __init panel_init_module(void)
break;
case PANEL_PROFILE_NEW:
/* serial, 2*16, new keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_NEW;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_KS0074;
+ selected_keypad_type = KEYPAD_TYPE_NEW;
+ selected_lcd_type = LCD_TYPE_KS0074;
break;
case PANEL_PROFILE_HANTRONIX:
/* 8 bits, 2*16 hantronix-like, no keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_NONE;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_HANTRONIX;
+ selected_keypad_type = KEYPAD_TYPE_NONE;
+ selected_lcd_type = LCD_TYPE_HANTRONIX;
break;
case PANEL_PROFILE_NEXCOM:
/* generic 8 bits, 2*16, nexcom keypad, eg. Nexcom. */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_NEXCOM;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_NEXCOM;
+ selected_keypad_type = KEYPAD_TYPE_NEXCOM;
+ selected_lcd_type = LCD_TYPE_NEXCOM;
break;
case PANEL_PROFILE_LARGE:
/* 8 bits, 2*40, old keypad */
- if (keypad_type == NOT_SET)
- keypad_type = KEYPAD_TYPE_OLD;
- if (lcd_type == NOT_SET)
- lcd_type = LCD_TYPE_OLD;
+ selected_keypad_type = KEYPAD_TYPE_OLD;
+ selected_lcd_type = LCD_TYPE_OLD;
break;
}
- lcd.enabled = (lcd_type > 0);
- keypad.enabled = (keypad_type > 0);
+ /*
+* Overwrite selection with module param values (both keypad and lcd),
+* where the deprecated params have lower prio.
+*/
+ if (keypad_enabled > -1)
+ selected_keypad_type = keypad_enabled;
+ if (keypad_type > -1)
+ selected_keypad_type = keypad_type;
+
+ keypad.enabled = (selected_keypad_type > 0);
+
+ if (lcd_enabled > -1)
+ selected_lcd_type = lcd_enabled;
+ if (lcd_type > -1)
+ selected_lcd_type = lcd_type;
+
+ lcd.enabled = (selec
[PATCH v3 9/9] staging: panel: Move LCD-related state into struct lcd
Move more or less all LCD-related state into struct lcd
in order to get better cohesion; use bool instead of int
where it makes sense.
Signed-off-by: Mariusz Gorski
Acked-by: Willy Tarreau
---
drivers/staging/panel/panel.c | 255 ++
1 file changed, 134 insertions(+), 121 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 19f6767..98325b7 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -225,12 +225,20 @@ static wait_queue_head_t keypad_read_wait;
/* lcd-specific variables */
static struct {
bool enabled;
+ bool initialized;
+ bool must_clear;
+
+ /* TODO: use bool here? */
+ char left_shift;
+
int height;
int width;
int bwidth;
int hwidth;
int charset;
int proto;
+ int light_tempo;
+
/* TODO: use union here? */
struct {
int e;
@@ -240,22 +248,26 @@ static struct {
int da;
int bl;
} pins;
+
+ /* contains the LCD config state */
+ unsigned long int flags;
+
+ /* Contains the LCD X and Y offset */
+ struct {
+ unsigned long int x;
+ unsigned long int y;
+ } addr;
+
+ /* Current escape sequence and it's length or -1 if outside */
+ struct {
+ char buf[LCD_ESCAPE_LEN + 1];
+ int len;
+ } esc_seq;
} lcd;
/* Needed only for init */
static int selected_lcd_type = NOT_SET;
-/* contains the LCD config state */
-static unsigned long int lcd_flags;
-/* contains the LCD X offset */
-static unsigned long int lcd_addr_x;
-/* contains the LCD Y offset */
-static unsigned long int lcd_addr_y;
-/* current escape sequence, 0 terminated */
-static char lcd_escape[LCD_ESCAPE_LEN + 1];
-/* not in escape state. >=0 = escape cmd len */
-static int lcd_escape_len = -1;
-
/*
* Bit masks to convert LCD signals to parallel port outputs.
* _d_ are values for data port, _c_ are for control port.
@@ -438,13 +450,8 @@ static atomic_t keypad_available = ATOMIC_INIT(1);
static struct pardevice *pprt;
-static int lcd_initialized;
static int keypad_initialized;
-static int light_tempo;
-
-static char lcd_must_clear;
-static char lcd_left_shift;
static char init_in_progress;
static void (*lcd_write_cmd)(int);
@@ -880,23 +887,23 @@ static void lcd_write_data_tilcd(int data)
static void lcd_gotoxy(void)
{
lcd_write_cmd(0x80 /* set DDRAM address */
- | (lcd_addr_y ? lcd.hwidth : 0)
+ | (lcd.addr.y ? lcd.hwidth : 0)
/* we force the cursor to stay at the end of the
line if it wants to go farther */
- | ((lcd_addr_x < lcd.bwidth) ? lcd_addr_x &
+ | ((lcd.addr.x < lcd.bwidth) ? lcd.addr.x &
(lcd.hwidth - 1) : lcd.bwidth - 1));
}
static void lcd_print(char c)
{
- if (lcd_addr_x < lcd.bwidth) {
+ if (lcd.addr.x < lcd.bwidth) {
if (lcd_char_conv != NULL)
c = lcd_char_conv[(unsigned char)c];
lcd_write_data(c);
- lcd_addr_x++;
+ lcd.addr.x++;
}
/* prevents the cursor from wrapping onto the next line */
- if (lcd_addr_x == lcd.bwidth)
+ if (lcd.addr.x == lcd.bwidth)
lcd_gotoxy();
}
@@ -905,8 +912,8 @@ static void lcd_clear_fast_s(void)
{
int pos;
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
spin_lock_irq(&pprt_lock);
@@ -918,8 +925,8 @@ static void lcd_clear_fast_s(void)
}
spin_unlock_irq(&pprt_lock);
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
}
@@ -928,8 +935,8 @@ static void lcd_clear_fast_p8(void)
{
int pos;
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
spin_lock_irq(&pprt_lock);
@@ -956,8 +963,8 @@ static void lcd_clear_fast_p8(void)
}
spin_unlock_irq(&pprt_lock);
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
}
@@ -966,8 +973,8 @@ static void lcd_clear_fast_tilcd(void)
{
int pos;
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
spin_lock_irq(&pprt_lock);
@@ -979,8 +986,8 @@ static void lcd_clear_fast_tilcd(void)
spin_unlock_irq(&pprt_lock);
- lcd_addr_x = 0;
- lcd_addr_y = 0;
+ lcd.addr.x = 0;
+ lcd.addr.y = 0;
lcd_gotoxy();
}
@@ -988,15 +995,15 @@ static void lcd_clear_fast_tilcd(void)
static void lcd_clear_display(void)
{
lcd_
[PATCH v3 7/9] staging: panel: Refactor LCD init code
Rework lcd_init method to make it a little bit more clear about
the precedence of the params, move LCD geometry and pins layout
to the LCD struct and thus make the LCD-related module params
effectively read-only.
Signed-off-by: Mariusz Gorski
Acked-by: Willy Tarreau
---
drivers/staging/panel/panel.c | 304 ++
1 file changed, 163 insertions(+), 141 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 5868a28..0bdb447 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -225,6 +225,21 @@ static wait_queue_head_t keypad_read_wait;
/* lcd-specific variables */
static struct {
bool enabled;
+ int height;
+ int width;
+ int bwidth;
+ int hwidth;
+ int charset;
+ int proto;
+ /* TODO: use union here? */
+ struct {
+ int e;
+ int rs;
+ int rw;
+ int cl;
+ int da;
+ int bl;
+ } pins;
} lcd;
/* Needed only for init */
@@ -766,7 +781,7 @@ static void lcd_send_serial(int byte)
/* turn the backlight on or off */
static void lcd_backlight(int on)
{
- if (lcd_bl_pin == PIN_NONE)
+ if (lcd.pins.bl == PIN_NONE)
return;
/* The backlight is activated by setting the AUTOFEED line to +5V */
@@ -865,23 +880,23 @@ static void lcd_write_data_tilcd(int data)
static void lcd_gotoxy(void)
{
lcd_write_cmd(0x80 /* set DDRAM address */
- | (lcd_addr_y ? lcd_hwidth : 0)
+ | (lcd_addr_y ? lcd.hwidth : 0)
/* we force the cursor to stay at the end of the
line if it wants to go farther */
- | ((lcd_addr_x < lcd_bwidth) ? lcd_addr_x &
-(lcd_hwidth - 1) : lcd_bwidth - 1));
+ | ((lcd_addr_x < lcd.bwidth) ? lcd_addr_x &
+(lcd.hwidth - 1) : lcd.bwidth - 1));
}
static void lcd_print(char c)
{
- if (lcd_addr_x < lcd_bwidth) {
+ if (lcd_addr_x < lcd.bwidth) {
if (lcd_char_conv != NULL)
c = lcd_char_conv[(unsigned char)c];
lcd_write_data(c);
lcd_addr_x++;
}
/* prevents the cursor from wrapping onto the next line */
- if (lcd_addr_x == lcd_bwidth)
+ if (lcd_addr_x == lcd.bwidth)
lcd_gotoxy();
}
@@ -895,7 +910,7 @@ static void lcd_clear_fast_s(void)
lcd_gotoxy();
spin_lock_irq(&pprt_lock);
- for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+ for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
lcd_send_serial(0x5F); /* R/W=W, RS=1 */
lcd_send_serial(' ' & 0x0F);
lcd_send_serial((' ' >> 4) & 0x0F);
@@ -918,7 +933,7 @@ static void lcd_clear_fast_p8(void)
lcd_gotoxy();
spin_lock_irq(&pprt_lock);
- for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+ for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');
@@ -956,7 +971,7 @@ static void lcd_clear_fast_tilcd(void)
lcd_gotoxy();
spin_lock_irq(&pprt_lock);
- for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+ for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');
udelay(60);
@@ -981,7 +996,7 @@ static void lcd_clear_display(void)
static void lcd_init_display(void)
{
- lcd_flags = ((lcd_height > 1) ? LCD_FLAG_N : 0)
+ lcd_flags = ((lcd.height > 1) ? LCD_FLAG_N : 0)
| LCD_FLAG_D | LCD_FLAG_C | LCD_FLAG_B;
long_sleep(20); /* wait 20 ms after power-up for the paranoid */
@@ -1095,17 +1110,17 @@ static inline int handle_lcd_special_code(void)
case 'l': /* Shift Cursor Left */
if (lcd_addr_x > 0) {
/* back one char if not at end of line */
- if (lcd_addr_x < lcd_bwidth)
+ if (lcd_addr_x < lcd.bwidth)
lcd_write_cmd(0x10);
lcd_addr_x--;
}
processed = 1;
break;
case 'r': /* shift cursor right */
- if (lcd_addr_x < lcd_width) {
+ if (lcd_addr_x < lcd.width) {
/* allow the cursor to pass the end of the line */
if (lcd_addr_x <
- (lcd_bwidth - 1))
+ (lcd.bwidth - 1))
lcd_write_cmd(0x14);
lcd_addr_x++;
}
@@ -1124,7 +1139,7 @@ static inline int handle_lcd_special_code(void)
case 'k': { /* kill
[PATCH v3 1/9] staging: panel: Set default parport module param value
Set default parport module param value to DEFAULT_PARPORT so that
a if-block can be avoided.
Signed-off-by: Mariusz Gorski
Acked-by: Willy Tarreau
---
drivers/staging/panel/panel.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index c6eeddf..3dd318a 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -429,7 +429,7 @@ static struct timer_list scan_timer;
MODULE_DESCRIPTION("Generic parallel port LCD/Keypad driver");
-static int parport = -1;
+static int parport = DEFAULT_PARPORT;
module_param(parport, int, );
MODULE_PARM_DESC(parport, "Parallel port index (0=lpt1, 1=lpt2, ...)");
@@ -2231,9 +2231,6 @@ static int panel_init(void)
if (lcd_type < 0)
lcd_type = lcd_enabled;
- if (parport < 0)
- parport = DEFAULT_PARPORT;
-
/* take care of an eventual profile */
switch (profile) {
case PANEL_PROFILE_CUSTOM:
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] mmc: rtsx: add support for sdio card
On 11/27/2014 11:43 PM, Dan Carpenter wrote: >> +int stat_idx = sd_status_index(rsp_type); > I have always hated this terrible pointer math. 5 is relative to > pcr->host_cmds_ptr + 1. It's a mess... 5 mean CRC7 offset of Response R1, see SD spec V3.01 Page 82. 4.9.1 R1 (normal response command). And we have to +1 to skip unused/undefined data. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] mfd: rtsx: add func to split u32 into register
On 11/27/2014 11:23 PM, Dan Carpenter wrote:
>> +static inline void rtsx_pci_write_be32(struct rtsx_pcr *pcr, u16 reg, u32
>> val)
>> >+{
>> >+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val >> 24);
>> >+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 16);
>> >+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 8);
>> >+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val);
> This assumes the cpu is little endian. First convert to big endian
> using cpu_to_be32() and then write it out.
>
> __be32 be_val = cpu_to_be32()
>
> rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, be_val);
> rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, be_val >> 8);
> rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, be_val >> 16);
> rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, be_val >> 24);
>
> (Written hurredly in my mail client. May be wrong).
>
I think we better not use cpu_to_be32() here, leave the work to caller
may be better.
eg, in sd_ops.c the cmd.arg is constructed bit by bit, we can put the right
byte to the right register by shift, so the endian check is not need.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] mfd: rtsx: add func to split u32 into register
From: Micky Ching
Add helper function to write u32 to registers, if we want to put u32
value to 4 continuous register, this can help us reduce tedious work.
Signed-off-by: Micky Ching
---
include/linux/mfd/rtsx_pci.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h
index 74346d5..9234449 100644
--- a/include/linux/mfd/rtsx_pci.h
+++ b/include/linux/mfd/rtsx_pci.h
@@ -558,6 +558,7 @@
#define SD_SAMPLE_POINT_CTL0xFDA7
#define SD_PUSH_POINT_CTL 0xFDA8
#define SD_CMD00xFDA9
+#define SD_CMD_START 0x40
#define SD_CMD10xFDAA
#define SD_CMD20xFDAB
#define SD_CMD30xFDAC
@@ -967,4 +968,12 @@ static inline u8 *rtsx_pci_get_cmd_data(struct rtsx_pcr
*pcr)
return (u8 *)(pcr->host_cmds_ptr);
}
+static inline void rtsx_pci_write_be32(struct rtsx_pcr *pcr, u16 reg, u32 val)
+{
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val >> 24);
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 16);
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 8);
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val);
+}
+
#endif
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] mmc: rtsx: add support for sdio card
From: Micky Ching
Add support for sdio card by SD interface. The main change is data
transfer mode, When read data, host wait data transfer while command
start. When write data, host will start data transfer after command get
response. The transfer mode modify can be applied both for SD/MMC card
and sdio card.
Signed-off-by: Micky Ching
---
drivers/mmc/host/rtsx_pci_sdmmc.c | 523 +-
1 file changed, 288 insertions(+), 235 deletions(-)
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c
b/drivers/mmc/host/rtsx_pci_sdmmc.c
index c70b602..b780779 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -71,30 +72,79 @@ static inline void sd_clear_error(struct realtek_pci_sdmmc
*host)
}
#ifdef DEBUG
-static void sd_print_debug_regs(struct realtek_pci_sdmmc *host)
+static void dump_reg_range(struct realtek_pci_sdmmc *host, u16 start, u16 end)
{
- struct rtsx_pcr *pcr = host->pcr;
- u16 i;
- u8 *ptr;
+ u16 len = end - start + 1;
+ int i;
+ u8 data[8];
- /* Print SD host internal registers */
- rtsx_pci_init_cmd(pcr);
- for (i = 0xFDA0; i <= 0xFDAE; i++)
- rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
- for (i = 0xFD52; i <= 0xFD69; i++)
- rtsx_pci_add_cmd(pcr, READ_REG_CMD, i, 0, 0);
- rtsx_pci_send_cmd(pcr, 100);
-
- ptr = rtsx_pci_get_cmd_data(pcr);
- for (i = 0xFDA0; i <= 0xFDAE; i++)
- dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
- for (i = 0xFD52; i <= 0xFD69; i++)
- dev_dbg(sdmmc_dev(host), "0x%04X: 0x%02x\n", i, *(ptr++));
+ if (!data)
+ return;
+
+ for (i = 0; i < len; i += 8) {
+ int j;
+ int n = min(8, len - i);
+
+ memset(&data, 0, sizeof(data));
+ for (j = 0; j < n; j++)
+ rtsx_pci_read_register(host->pcr, start + i + j,
+ data + j);
+ dev_dbg(sdmmc_dev(host), "0x%04X(%d): %8ph\n", start, n, data);
+ }
+}
+
+static void sd_print_debug_regs(struct realtek_pci_sdmmc *host)
+{
+ dump_reg_range(host, 0xFDA0, 0xFDB3);
+ dump_reg_range(host, 0xFD52, 0xFD69);
}
#else
#define sd_print_debug_regs(host)
#endif /* DEBUG */
+static void sd_cmd_set_sd_cmd(struct rtsx_pcr *pcr, struct mmc_command *cmd)
+{
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_CMD0, 0xFF, SD_CMD_START |
cmd->opcode);
+ rtsx_pci_write_be32(pcr, SD_CMD1, cmd->arg);
+}
+
+static void sd_cmd_set_data_len(struct rtsx_pcr *pcr, u16 blocks, u16 blksz)
+{
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_BLOCK_CNT_L, 0xFF, blocks);
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_BLOCK_CNT_H, 0xFF, blocks >> 8);
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_BYTE_CNT_L, 0xFF, blksz);
+ rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_BYTE_CNT_H, 0xFF, blksz >> 8);
+}
+
+static int sd_response_type(struct mmc_command *cmd)
+{
+ switch (mmc_resp_type(cmd)) {
+ case MMC_RSP_NONE:
+ return SD_RSP_TYPE_R0;
+ case MMC_RSP_R1:
+ return SD_RSP_TYPE_R1;
+ case MMC_RSP_R1 & ~MMC_RSP_CRC:
+ return SD_RSP_TYPE_R1 | SD_NO_CHECK_CRC7;
+ case MMC_RSP_R1B:
+ return SD_RSP_TYPE_R1b;
+ case MMC_RSP_R2:
+ return SD_RSP_TYPE_R2;
+ case MMC_RSP_R3:
+ return SD_RSP_TYPE_R3;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sd_status_index(int resp_type)
+{
+ if (resp_type == SD_RSP_TYPE_R0)
+ return 0;
+ else if (resp_type == SD_RSP_TYPE_R2)
+ return 16;
+
+ return 5;
+}
/*
* sd_pre_dma_transfer - do dma_map_sg() or using cookie
*
@@ -166,123 +216,6 @@ static void sdmmc_post_req(struct mmc_host *mmc, struct
mmc_request *mrq,
data->host_cookie = 0;
}
-static int sd_read_data(struct realtek_pci_sdmmc *host, u8 *cmd, u16 byte_cnt,
- u8 *buf, int buf_len, int timeout)
-{
- struct rtsx_pcr *pcr = host->pcr;
- int err, i;
- u8 trans_mode;
-
- dev_dbg(sdmmc_dev(host), "%s: SD/MMC CMD%d\n", __func__, cmd[0] - 0x40);
-
- if (!buf)
- buf_len = 0;
-
- if ((cmd[0] & 0x3F) == MMC_SEND_TUNING_BLOCK)
- trans_mode = SD_TM_AUTO_TUNING;
- else
- trans_mode = SD_TM_NORMAL_READ;
-
- rtsx_pci_init_cmd(pcr);
-
- for (i = 0; i < 5; i++)
- rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_CMD0 + i, 0xFF, cmd[i]);
-
- rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_BYTE_CNT_L, 0xFF, (u8)byte_cnt);
- rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_BYTE_CNT_H,
- 0xFF, (u8)(byte_cnt >> 8));
- rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_BLOCK_CNT_L, 0xFF, 1);
-
[PATCH v2 0/2] mmc: rtsx: add support for sdio card
From: Micky Ching v2: rtsx_pci.h: - remove unused rtsx_pci_write_le32 - add SD_CMD_START rtsx_pci_sdmmc.c: - dump_reg_range - alloc data on stack - remove forward declaration - use SD_CMD_START replace magic number 0x40 - move initialize assignment to error handling This patch is used to change transfer mode for sdio card support by SD interface. Micky Ching (2): mfd: rtsx: add func to split u32 into register mmc: rtsx: add support for sdio card drivers/mmc/host/rtsx_pci_sdmmc.c | 523 +- include/linux/mfd/rtsx_pci.h | 9 + 2 files changed, 297 insertions(+), 235 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] hv: hv_fcopy: drop the obsolete message on transfer failure
On Thu, Nov 27, 2014 at 9:09 PM, Dexuan Cui wrote:
In the case the user-space daemon crashes, hangs or is killed, we
need to down the semaphore, otherwise, after the daemon starts next
time, the obsolete data in fcopy_transaction.message or
fcopy_transaction.fcopy_msg will be used immediately.
Cc: Jason Wang
Cc: Vitaly Kuznetsov
Cc: K. Y. Srinivasan
Signed-off-by: Dexuan Cui
---
v2: I removed the "FCP" prefix as Greg asked.
I also updated the output message a little:
"FCP: failed to acquire the semaphore" -->
"can not acquire the semaphore: it is benign"
v3: I added the code in fcopy_release() as Jason Wang suggested.
I removed the pr_debug (it isn't so meaningful)and added a
comment instead.
drivers/hv/hv_fcopy.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 23b2ce2..faa6ba6 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -86,6 +86,18 @@ static void fcopy_work_func(struct work_struct
*dummy)
* process the pending transaction.
*/
fcopy_respond_to_host(HV_E_FAIL);
+
+ /* In the case the user-space daemon crashes, hangs or is killed, we
+ * need to down the semaphore, otherwise, after the daemon starts
next
+* time, the obsolete data in fcopy_transaction.message or
+* fcopy_transaction.fcopy_msg will be used immediately.
+*
+ * NOTE: fcopy_read() happens to get the semaphore (very rare)?
We're
+* still OK, because we've reported the failure to the host.
+*/
+ if (down_trylock(&fcopy_transaction.read_sema))
+ ;
Sorry, I'm not quite understand how if () ; can help here.
Btw, a question not relate to this patch.
What happens if a daemon is resume from SIGSTOP and expires the check
here?
+
}
static int fcopy_handle_handshake(u32 version)
@@ -351,6 +363,13 @@ static int fcopy_release(struct inode *inode,
struct file *f)
*/
in_hand_shake = true;
opened = false;
+
+ if (cancel_delayed_work_sync(&fcopy_work)) {
+ /* We haven't up()-ed the semaphore(very rare)? */
+ if (down_trylock(&fcopy_transaction.read_sema))
+ ;
And this.
+ fcopy_respond_to_host(HV_E_FAIL);
+ }
return 0;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers:Staging:rtl8188eu:hal:usb_halinit.c: Added blank line after declarations
On Thu, Nov 27, 2014 at 08:13:03PM +0530, Anjana Sasindran wrote:
> This patch fixes the five checkpatch.pl warnings:
>
> WARNING:Missing a blank line after declaration
>
> Signed-off-by: Anjana Sasindran
> ---
> drivers/staging/rtl8188eu/hal/usb_halinit.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c
> b/drivers/staging/rtl8188eu/hal/usb_halinit.c
> index 14650e9..439828c 100644
> --- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
> +++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
> @@ -1931,6 +1931,7 @@ GetHalDefVar8188EUsb(
> case HW_DEF_RA_INFO_DUMP:
> {
> u8 entry_id = *((u8 *)pValue);
> +
> if (check_fwstate(&Adapter->mlmepriv, _FW_LINKED)) {
> DBG_88E(" RA status check
> ===\n");
> DBG_88E("Mac_id:%d , RateID = %d, RAUseRate =
> 0x%08x, RateSGI = %d, DecisionRate = 0x%02x ,PTStage = %d\n",
> @@ -1946,6 +1947,7 @@ GetHalDefVar8188EUsb(
> case HW_DEF_ODM_DBG_FLAG:
> {
> struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
> +
> pr_info("dm_ocm->DebugComponents = 0x%llx\n",
> dm_ocm->DebugComponents);
> }
> break;
> @@ -1994,6 +1996,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter,
> enum hal_def_variable eV
> } else if (dm_func == 6) {/* turn on all dynamic func */
> if (!(podmpriv->SupportAbility &
> DYNAMIC_BB_DIG)) {
> struct rtw_dig *pDigTable =
> &podmpriv->DM_DigTable;
> +
> pDigTable->CurIGValue =
> usb_read8(Adapter, 0xc50);
> }
> podmpriv->SupportAbility =
> DYNAMIC_ALL_FUNC_ENABLE;
> @@ -2011,6 +2014,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter,
> enum hal_def_variable eV
> {
> u8 bRSSIDump = *((u8 *)pValue);
> struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
> +
> if (bRSSIDump)
> dm_ocm->DebugComponents =
> ODM_COMP_DIG|ODM_COMP_FA_CNT;
> else
> @@ -2021,7 +2025,9 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter,
> enum hal_def_variable eV
> {
> u64 DebugComponents = *((u64 *)pValue);
> struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
> +
> dm_ocm->DebugComponents = DebugComponents;
> +
any reason to give this blank line here? This is not a declaration.
thanks
sudip
> }
> break;
> default:
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
