[PATCH resend] ARM: at91/ide: remove unsused at91-ide Kconfig entry
Commit cf844751fb25e ("ARM: at91: drop ide driver in favor of the pata
one") removed the at91-ide driver but did not remove the Kconfig entry.
Signed-off-by: Johan Hovold
---
Resend of patch first posted mid-November with trivial added as CC. Perhaps it
can go in via the AT91-folks, otherwise?
Thanks,
Johan
drivers/ide/Kconfig | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig
index 5a26584..bfec4e6 100644
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -702,11 +702,6 @@ config BLK_DEV_IDE_TX4939
depends on SOC_TX4939
select BLK_DEV_IDEDMA_SFF
-config BLK_DEV_IDE_AT91
- tristate "Atmel AT91 (SAM9, CAP9, AT572D940HF) IDE support"
- depends on ARM && ARCH_AT91 && !ARCH_AT91RM9200 && !ARCH_AT91X40
- select IDE_TIMINGS
-
config BLK_DEV_IDE_ICSIDE
tristate "ICS IDE interface support"
depends on ARM && ARCH_ACORN
--
1.8.1.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/
Re: [PATCH resend] ARM: at91/ide: remove unsused at91-ide Kconfig entry
On Tue, Feb 5, 2013 at 2:59 PM, Jiri Kosina wrote:
> On Tue, 5 Feb 2013, Johan Hovold wrote:
>
>> Commit cf844751fb25e ("ARM: at91: drop ide driver in favor of the pata
>> one") removed the at91-ide driver but did not remove the Kconfig entry.
>>
>> Signed-off-by: Johan Hovold
>> ---
>>
>> Resend of patch first posted mid-November with trivial added as CC. Perhaps
>> it
>> can go in via the AT91-folks, otherwise?
>
> It doesn't seem to have reached triv...@kernel.org back then.
No, I forgot to add trivial last time.
> I am taking it now, thanks.
Thanks,
Johan
--
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/
Re: [PATCH 0/5] at91: atmel_lcdfb: regression fixes and cpu_is removal
On Sun, Feb 10, 2013 at 1:47 AM, Olof Johansson wrote:
> On Fri, Feb 08, 2013 at 05:35:13PM +0100, Nicolas Ferre wrote:
>> These patches fix a regression in 16-bpp support for older SOCs which
>> use IBGR:555 rather than BGR:565 pixel layout. Use SOC-type to
>> determine if the controller uses the intensity-bit and restore the
>> old layout in that case.
>>
>> The last patch is a removal of uses of cpu_is_() macros in
>> atmel_lcdfb with a platform-device-id table and static
>> configurations.
>>
>>
>> Patches from Johan Hovold taken from: "[PATCH 0/3] atmel_lcdfb: fix
>> 16-bpp regression" and "[PATCH v2 0/3] ARM: at91/avr32/atmel_lcdfb:
>> remove cpu_is macros" patch series to form a clean patch series with
>> my signature.
>>
>> Arnd, Olof, as it seems that old fbdev drivers are not so much
>> reviewed those days, can we take the decision to queue this material
>> through arm-soc with other AT91 drivers updates?
>
> It would be beneficial to get an ack from Florian. Was he involved in
> the review of the code that regressed 16-bpp support in the first
> place? When was the regression introduced?
In v3.4 by commit 787f9fd2328 ("atmel_lcdfb: support 16bit BGR:565 mode,
remove unsupported 15bit modes").
Johan
--
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/
Re: [RFC PATCH v2] rtc: rtc-at91rm9200: manage IMR depending on revision
On Tue, Apr 02, 2013 at 06:36:06PM +0200, Nicolas Ferre wrote: > Signed-off-by: Nicolas Ferre > --- > Hi again, > > Here is my latest revision of this fix. It depends on the patch that is > already > in Andrew's patch stack: "drivers-rtc-rtc-at91rm9200c-add-dt-support.patch". That is a problem, as the patch in Andrew's stack is not (and should not) be marked for stable. Hence this patch cannot be applied to the stable trees and it won't even apply to 3.9-rc. But there's more: The offending patch introduced the races we have been discussion while attempting to add support for the sam9x5 with the broken hardware register. But that family cannot be used without DT-support, which the driver currently does not support. Hence, we added a workaround (and introduced a regression by mistake), while adding support for a SoC which still could not use the driver. [ For example, the sam9x5 RTC-base register address can only be supplied from DT. ] I think the only reasonable thing to do is to revert the patch and add whatever version of the work-around on top of the device-tree support when that is added to the driver (hence, earliest v3.10). > I now use a different compatibility string to figure out what is the IP > revision that has the "boggus IMR" error. I think this way to handle it > is much simpler than the "config" structure one from Johan. I wouldn't say it's much simpler. My solution is only more generic, but could of course also be reduced to "set a flag if compatible matches sam9x5". > The small number of line changed and the "single patch" nature of it > make me think that it will be easier to send upstream and in the > "stable" trees... Unfortunately, the 130-line diff isn't very small. In fact, it violates the stable-kernel guide line of <100 lines. And as noted above, it depends on another patch which adds DT-support (which is a new feature and not a fix). But the fundamental problem remains: it does not fix anything which was working before the first work-around patch introduced the regression. I think this is a clear case where we need to revert. > Please give feedback, but moreover, I would like to know if you (Johan and > Douglas) > agree to give your "Signed-off-by" line because this patch is certainly > inspired by your comments, code and reviews. > > Thank you for your help. Best regards, > > .../bindings/rtc/atmel,at91rm9200-rtc.txt | 3 +- > drivers/rtc/rtc-at91rm9200.c | 126 > - > drivers/rtc/rtc-at91rm9200.h | 1 + > 3 files changed, 101 insertions(+), 29 deletions(-) > > diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > index 2a3feab..9b87053 100644 > --- a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > +++ b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > @@ -1,7 +1,8 @@ > Atmel AT91RM9200 Real Time Clock > > Required properties: > -- compatible: should be: "atmel,at91rm9200-rtc" > +- compatible: should be: "atmel,at91rm9200-rtc", "atmel,at91sam9x5-rtc" or > + "atmel,at91sam9n12-rtc". Also at91sam9g45 and at91sam9rl use this driver. As seems to be the case for other peripherals, I suggest we use "atmel,at91sam9x5-rtc" for sam9x5 and "atmel,at91rm9200-rtc" for the other SoCs, that is, the least (and first) common denominator. Either way, there's not need to add at91sam9n12-rtc in this patch. > - reg: physical base address of the controller and length of memory mapped >region. > - interrupts: rtc alarm/event interrupt I'll respond to this mail with a revert-patch, and an updated RFC-series based on top of the DT-patch in Andrew's queue. Thanks, Johan -- 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/
[PATCH] Revert "drivers/rtc/rtc-at91rm9200.c: use a variable for storing IMR"
This reverts commit 0ef1594c017521ea89278e80fe3f80dafb17abde.
This patch introduced a few races which cannot be easily fixed with a
small follow-up patch. Furthermore, the SoC with the broken hardware
register, which this patch intended to add support for, can only be used
with device trees, which this driver currently does not support.
Cc: stable
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 50 +---
drivers/rtc/rtc-at91rm9200.h | 1 +
2 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 0a9f27e..434ebc3 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -44,7 +44,6 @@ static DECLARE_COMPLETION(at91_rtc_updated);
static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
static int irq;
-static u32 at91_rtc_imr;
/*
* Decode time/date into rtc_time structure
@@ -109,11 +108,9 @@ static int at91_rtc_settime(struct device *dev, struct
rtc_time *tm)
cr = at91_rtc_read(AT91_RTC_CR);
at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | AT91_RTC_UPDTIM);
- at91_rtc_imr |= AT91_RTC_ACKUPD;
at91_rtc_write(AT91_RTC_IER, AT91_RTC_ACKUPD);
wait_for_completion(&at91_rtc_updated); /* wait for ACKUPD interrupt */
at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD);
- at91_rtc_imr &= ~AT91_RTC_ACKUPD;
at91_rtc_write(AT91_RTC_TIMR,
bin2bcd(tm->tm_sec) << 0
@@ -145,7 +142,7 @@ static int at91_rtc_readalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
tm->tm_year = at91_alarm_year - 1900;
- alrm->enabled = (at91_rtc_imr & AT91_RTC_ALARM)
+ alrm->enabled = (at91_rtc_read(AT91_RTC_IMR) & AT91_RTC_ALARM)
? 1 : 0;
dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -171,7 +168,6 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm.tm_sec = alrm->time.tm_sec;
at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
- at91_rtc_imr &= ~AT91_RTC_ALARM;
at91_rtc_write(AT91_RTC_TIMALR,
bin2bcd(tm.tm_sec) << 0
| bin2bcd(tm.tm_min) << 8
@@ -184,7 +180,6 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
if (alrm->enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_imr |= AT91_RTC_ALARM;
at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
}
@@ -201,12 +196,9 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
if (enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_imr |= AT91_RTC_ALARM;
at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
- } else {
+ } else
at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
- at91_rtc_imr &= ~AT91_RTC_ALARM;
- }
return 0;
}
@@ -215,10 +207,12 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
*/
static int at91_rtc_proc(struct device *dev, struct seq_file *seq)
{
+ unsigned long imr = at91_rtc_read(AT91_RTC_IMR);
+
seq_printf(seq, "update_IRQ\t: %s\n",
- (at91_rtc_imr & AT91_RTC_ACKUPD) ? "yes" : "no");
+ (imr & AT91_RTC_ACKUPD) ? "yes" : "no");
seq_printf(seq, "periodic_IRQ\t: %s\n",
- (at91_rtc_imr & AT91_RTC_SECEV) ? "yes" : "no");
+ (imr & AT91_RTC_SECEV) ? "yes" : "no");
return 0;
}
@@ -233,7 +227,7 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
unsigned int rtsr;
unsigned long events = 0;
- rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_imr;
+ rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read(AT91_RTC_IMR);
if (rtsr) { /* this interrupt is shared! Is it ours? */
if (rtsr & AT91_RTC_ALARM)
events |= (RTC_AF | RTC_IRQF);
@@ -297,7 +291,6 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
AT91_RTC_SECEV | AT91_RTC_TIMEV |
AT91_RTC_CALEV);
- at91_rtc_imr = 0;
ret = request_irq(irq, at91_rtc_interrupt,
IRQF_SHARED,
@@ -336,7 +329,6 @@ static int __exit at91_rtc_remove(struct platform_device
*pdev)
at91_rtc_write(AT91_RTC_IDR, AT91_RTC
[RFC v2 1/4] rtc-at91rm9200: add configuration support
Add configuration support which can be used to implement SoC-specific
workarounds for broken hardware.
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 42 --
1 file changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 79233d0..26e560c 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -42,6 +42,10 @@
#define AT91_RTC_EPOCH 1900UL /* just like arch/arm/common/rtctime.c
*/
+struct at91_rtc_config {
+};
+
+static const struct at91_rtc_config *at91_rtc_config;
static DECLARE_COMPLETION(at91_rtc_updated);
static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
@@ -250,6 +254,34 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
*dev_id)
return IRQ_NONE;/* not handled */
}
+static const struct at91_rtc_config at91rm9200_config = {
+};
+
+static const struct of_device_id at91_rtc_dt_ids[] = {
+ {
+ .compatible = "atmel,at91rm9200-rtc",
+ .data = &at91rm9200_config,
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
+
+static const struct at91_rtc_config *
+at91_rtc_get_config(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+
+ if (pdev->dev.of_node) {
+ match = of_match_node(at91_rtc_dt_ids, pdev->dev.of_node);
+ if (!match)
+ return NULL;
+ return (const struct at91_rtc_config *)match->data;
+ }
+
+ return &at91rm9200_config;
+}
+
static const struct rtc_class_ops at91_rtc_ops = {
.read_time = at91_rtc_readtime,
.set_time = at91_rtc_settime,
@@ -268,6 +300,10 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
struct resource *regs;
int ret = 0;
+ at91_rtc_config = at91_rtc_get_config(pdev);
+ if (!at91_rtc_config)
+ return -ENODEV;
+
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
dev_err(&pdev->dev, "no mmio resource defined\n");
@@ -383,12 +419,6 @@ static const struct dev_pm_ops at91_rtc_pm = {
#define at91_rtc_pm_ptrNULL
#endif
-static const struct of_device_id at91_rtc_dt_ids[] = {
- { .compatible = "atmel,at91rm9200-rtc" },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
-
static struct platform_driver at91_rtc_driver = {
.remove = __exit_p(at91_rtc_remove),
.driver = {
--
1.8.1.5
--
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/
[RFC v2 4/4] rtc-at91rm9200: add support for at91sam9x5
Add support for the at91sam9x5-family which must use the shadow
interrupt mask due to a hardware issue.
Signed-off-by: Johan Hovold
---
Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt | 2 +-
drivers/rtc/rtc-at91rm9200.c | 7 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
index 2a3feab..34c1505 100644
--- a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
@@ -1,7 +1,7 @@
Atmel AT91RM9200 Real Time Clock
Required properties:
-- compatible: should be: "atmel,at91rm9200-rtc"
+- compatible: should be: "atmel,at91rm9200-rtc" or "atmel,at91sam9x5-rtc"
- reg: physical base address of the controller and length of memory mapped
region.
- interrupts: rtc alarm/event interrupt
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index ac5f8ed..7c19390 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -320,11 +320,18 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
*dev_id)
static const struct at91_rtc_config at91rm9200_config = {
};
+static const struct at91_rtc_config at91sam9x5_config = {
+ .use_shadow_imr = true,
+};
+
static const struct of_device_id at91_rtc_dt_ids[] = {
{
.compatible = "atmel,at91rm9200-rtc",
.data = &at91rm9200_config,
}, {
+ .compatible = "atmel,at91sam9x5-rtc",
+ .data = &at91sam9x5_config,
+ }, {
/* sentinel */
}
};
--
1.8.1.5
--
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/
[RFC v2 0/4] rtc-at91rm9200: add support for at91sam9x5
This v2 of my version of the work-around for the buggy hardware
register on sam9x5, based on top of the revert of commit 0ef1594c017
("drivers/rtc/rtc-at91rm9200.c: use a variable for storing IMR") and the
DT-support patch in Andrew's queue.
If preferred, the generic configuration support added in the first patch
could easily be reduced to a single flag set based on the
compatible-property matching sam9x5.
Johan
v2
- rebase on top of DT-support patch by Joachim Eastwood
- add missing brace in DT-id table
Johan Hovold (4):
rtc-at91rm9200: add configuration support
rtc-at91rm9200: refactor interrupt-register handling
rtc-at91rm9200: add shadow interrupt mask
rtc-at91rm9200: add support for at91sam9x5
.../bindings/rtc/atmel,at91rm9200-rtc.txt | 2 +-
drivers/rtc/rtc-at91rm9200.c | 140 ++---
2 files changed, 121 insertions(+), 21 deletions(-)
--
1.8.1.5
--
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/
[RFC v2 2/4] rtc-at91rm9200: refactor interrupt-register handling
Add accessors for the interrupt register.
This will allow us to easily add a shadow interrupt-mask register to
use on SoCs where the interrupt-mask register cannot be used.
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 43 +--
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 26e560c..46576ff 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -51,6 +51,21 @@ static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
static int irq;
+static void at91_rtc_write_ier(u32 mask)
+{
+ at91_rtc_write(AT91_RTC_IER, mask);
+}
+
+static void at91_rtc_write_idr(u32 mask)
+{
+ at91_rtc_write(AT91_RTC_IDR, mask);
+}
+
+static u32 at91_rtc_read_imr(void)
+{
+ return at91_rtc_read(AT91_RTC_IMR);
+}
+
/*
* Decode time/date into rtc_time structure
*/
@@ -114,9 +129,9 @@ static int at91_rtc_settime(struct device *dev, struct
rtc_time *tm)
cr = at91_rtc_read(AT91_RTC_CR);
at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | AT91_RTC_UPDTIM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ACKUPD);
+ at91_rtc_write_ier(AT91_RTC_ACKUPD);
wait_for_completion(&at91_rtc_updated); /* wait for ACKUPD interrupt */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD);
+ at91_rtc_write_idr(AT91_RTC_ACKUPD);
at91_rtc_write(AT91_RTC_TIMR,
bin2bcd(tm->tm_sec) << 0
@@ -148,7 +163,7 @@ static int at91_rtc_readalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
tm->tm_year = at91_alarm_year - 1900;
- alrm->enabled = (at91_rtc_read(AT91_RTC_IMR) & AT91_RTC_ALARM)
+ alrm->enabled = (at91_rtc_read_imr() & AT91_RTC_ALARM)
? 1 : 0;
dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -173,7 +188,7 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm.tm_min = alrm->time.tm_min;
tm.tm_sec = alrm->time.tm_sec;
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+ at91_rtc_write_idr(AT91_RTC_ALARM);
at91_rtc_write(AT91_RTC_TIMALR,
bin2bcd(tm.tm_sec) << 0
| bin2bcd(tm.tm_min) << 8
@@ -186,7 +201,7 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
if (alrm->enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+ at91_rtc_write_ier(AT91_RTC_ALARM);
}
dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -202,9 +217,9 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
if (enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+ at91_rtc_write_ier(AT91_RTC_ALARM);
} else
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+ at91_rtc_write_idr(AT91_RTC_ALARM);
return 0;
}
@@ -213,7 +228,7 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
*/
static int at91_rtc_proc(struct device *dev, struct seq_file *seq)
{
- unsigned long imr = at91_rtc_read(AT91_RTC_IMR);
+ unsigned long imr = at91_rtc_read_imr();
seq_printf(seq, "update_IRQ\t: %s\n",
(imr & AT91_RTC_ACKUPD) ? "yes" : "no");
@@ -233,7 +248,7 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
unsigned int rtsr;
unsigned long events = 0;
- rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read(AT91_RTC_IMR);
+ rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read_imr();
if (rtsr) { /* this interrupt is shared! Is it ours? */
if (rtsr & AT91_RTC_ALARM)
events |= (RTC_AF | RTC_IRQF);
@@ -326,7 +341,7 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
at91_rtc_write(AT91_RTC_MR, 0); /* 24 hour mode */
/* Disable all interrupts */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+ at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
AT91_RTC_SECEV | AT91_RTC_TIMEV |
AT91_RTC_CALEV);
@@ -364,7 +379,7 @@ static int __exit at91_rtc_remove(struct platform_device
*pdev)
struct rtc_device *rtc = platform_get_drvdata(pdev);
/* Disable all interrupts */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+
[RFC v2 3/4] rtc-at91rm9200: add shadow interrupt mask
Add shadow interrupt-mask register which can be used on SoCs where the
actual hardware register is broken.
Note that some care needs to be taken to make sure the shadow mask
corresponds to the actual hardware state. The added overhead is not an
issue for the non-broken SoCs due to the relatively infrequent
interrupt-mask updates. We do, however, only use the shadow mask value
as a fall-back when it actually needed as there is still a theoretical
possibility that the mask is incorrect (see the code for details).
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 50 +++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 46576ff..ac5f8ed 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
@@ -43,6 +44,7 @@
#define AT91_RTC_EPOCH 1900UL /* just like arch/arm/common/rtctime.c
*/
struct at91_rtc_config {
+ bool use_shadow_imr;
};
static const struct at91_rtc_config *at91_rtc_config;
@@ -50,20 +52,66 @@ static DECLARE_COMPLETION(at91_rtc_updated);
static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
static int irq;
+static DEFINE_SPINLOCK(at91_rtc_lock);
+static u32 at91_rtc_shadow_imr;
static void at91_rtc_write_ier(u32 mask)
{
+ unsigned long flags;
+
+ /*
+* Lock needed to make sure shadow mask is updated before interrupts
+* are enabled.
+*/
+ spin_lock_irqsave(&at91_rtc_lock, flags);
+ at91_rtc_shadow_imr |= mask;
at91_rtc_write(AT91_RTC_IER, mask);
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
}
static void at91_rtc_write_idr(u32 mask)
{
+ unsigned long flags;
+
+ /*
+* Lock needed to make sure shadow mask is not updated before
+* interrupts have been disabled.
+*/
+ spin_lock_irqsave(&at91_rtc_lock, flags);
at91_rtc_write(AT91_RTC_IDR, mask);
+ /*
+* Register read back (of any RTC-register) needed to make sure
+* IDR-register write has reached the peripheral before updating
+* shadow mask.
+*
+* Note that there is still a possibility that the mask is updated
+* before interrupts have actually been disabled in hardware. The only
+* way to be certain would be to poll the IMR-register, which is is
+* the very register we are trying to emulate. The register read back
+* is a reasonable heuristic.
+*/
+ at91_rtc_read(AT91_RTC_SR);
+ at91_rtc_shadow_imr &= ~mask;
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
}
static u32 at91_rtc_read_imr(void)
{
- return at91_rtc_read(AT91_RTC_IMR);
+ unsigned long flags;
+ u32 mask;
+
+ if (at91_rtc_config->use_shadow_imr) {
+ /*
+* Lock not strictly necessary on UP.
+*/
+ spin_lock_irqsave(&at91_rtc_lock, flags);
+ mask = at91_rtc_shadow_imr;
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
+ } else {
+ mask = at91_rtc_read(AT91_RTC_IMR);
+ }
+
+ return mask;
}
/*
--
1.8.1.5
--
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/
Re: [RFC PATCH v2] rtc: rtc-at91rm9200: manage IMR depending on revision
On Wed, Apr 03, 2013 at 12:37:47PM +0200, Nicolas Ferre wrote: > On 04/03/2013 11:51 AM, Johan Hovold : > > On Tue, Apr 02, 2013 at 06:36:06PM +0200, Nicolas Ferre wrote: [...] > >> I now use a different compatibility string to figure out what is the IP > >> revision that has the "boggus IMR" error. I think this way to handle it > >> is much simpler than the "config" structure one from Johan. > > > > I wouldn't say it's much simpler. My solution is only more generic, but > > could of course also be reduced to "set a flag if compatible matches > > sam9x5". > > The advantage is precisely to avoid the need for a "flag". Only function > pointers that are changed in case of the compatible string matching. Yeah, you could do it that way. The overhead is negligible in either solution; mask updates are infrequent and the only difference when retrieving the mask would be to first check the flag. An advantage of using the config-struct would perhaps be that it is same mechanism used in i2c-at91 and atmel_lcdfb (in the arm-soc tree) to deal with SoC-quirks and is easily extended should need arise. The diffs of both solutions are also of roughly the same size. But I don't have any strong preference. You decide. [...] > >> diff --git > >> a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > >> b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > >> index 2a3feab..9b87053 100644 > >> --- a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > >> +++ b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt > >> @@ -1,7 +1,8 @@ > >> Atmel AT91RM9200 Real Time Clock > >> > >> Required properties: > >> -- compatible: should be: "atmel,at91rm9200-rtc" > >> +- compatible: should be: "atmel,at91rm9200-rtc", "atmel,at91sam9x5-rtc" or > >> + "atmel,at91sam9n12-rtc". > > > > Also at91sam9g45 and at91sam9rl use this driver. > > Yes, sure, I did not want to add every single user of the RTC... > > > As seems to be the case > > for other peripherals, I suggest we use "atmel,at91sam9x5-rtc" for > > sam9x5 and "atmel,at91rm9200-rtc" for the other SoCs, that is, the least > > (and first) common denominator. > > ... I was just following the habit of naming the changes in peripheral > revision by it first use in a SoC: > at91rm9200-rtc: from rm9200 up to 9g45 > at91sam9x5-rtc: sam9x5 only (with IMR issue) > at91sam9n12-rtc: fist SoC that corrects the IMR issue with a new IP > revision, until now and sama5d3 SoC Ah, ok. > > Either way, there's not need to add at91sam9n12-rtc in this patch. > > > >> - reg: physical base address of the controller and length of memory mapped > >>region. > >> - interrupts: rtc alarm/event interrupt > > > > I'll respond to this mail with a revert-patch, and an updated RFC-series > > based on top of the DT-patch in Andrew's queue. Thanks, Johan -- 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/
[PATCH] ARM: w1-gpio: fix erroneous gpio requests
Fix regression introduced by commit d2323cf773 ("onewire: w1-gpio: add
ext_pullup_enable pin in platform data") which added a gpio entry to the
platform data, but did not add the required initialisers to the board
files using it. Consequently, the driver would request gpio 0 at probe,
which could break other uses of the corresponding pin.
On AT91 requesting gpio 0 changes the pin muxing for PIOA0, which, for
instance, breaks SPI0 on at91sam9g20.
Cc: stable
Signed-off-by: Johan Hovold
---
Not sure whose tree this should go in through, so adding relevant maintainers
and authors as CC.
/Johan
arch/arm/mach-at91/board-foxg20.c| 1 +
arch/arm/mach-at91/board-stamp9g20.c | 1 +
arch/arm/mach-ixp4xx/vulcan-setup.c | 1 +
arch/arm/mach-pxa/raumfeld.c | 1 +
4 files changed, 4 insertions(+)
diff --git a/arch/arm/mach-at91/board-foxg20.c
b/arch/arm/mach-at91/board-foxg20.c
index 191d37c..1478294 100644
--- a/arch/arm/mach-at91/board-foxg20.c
+++ b/arch/arm/mach-at91/board-foxg20.c
@@ -176,6 +176,7 @@ static struct w1_gpio_platform_data w1_gpio_pdata = {
/* If you choose to use a pin other than PB16 it needs to be 3.3V */
.pin= AT91_PIN_PB16,
.is_open_drain = 1,
+ .ext_pullup_enable_pin = -EINVAL,
};
static struct platform_device w1_device = {
diff --git a/arch/arm/mach-at91/board-stamp9g20.c
b/arch/arm/mach-at91/board-stamp9g20.c
index 48a962b..58a6758 100644
--- a/arch/arm/mach-at91/board-stamp9g20.c
+++ b/arch/arm/mach-at91/board-stamp9g20.c
@@ -188,6 +188,7 @@ static struct spi_board_info portuxg20_spi_devices[] = {
static struct w1_gpio_platform_data w1_gpio_pdata = {
.pin= AT91_PIN_PA29,
.is_open_drain = 1,
+ .ext_pullup_enable_pin = -EINVAL,
};
static struct platform_device w1_device = {
diff --git a/arch/arm/mach-ixp4xx/vulcan-setup.c
b/arch/arm/mach-ixp4xx/vulcan-setup.c
index 2798f43..1dddc1b 100644
--- a/arch/arm/mach-ixp4xx/vulcan-setup.c
+++ b/arch/arm/mach-ixp4xx/vulcan-setup.c
@@ -163,6 +163,7 @@ static struct platform_device vulcan_max6369 = {
static struct w1_gpio_platform_data vulcan_w1_gpio_pdata = {
.pin= 14,
+ .ext_pullup_enable_pin = -EINVAL,
};
static struct platform_device vulcan_w1_gpio = {
diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
index 25b08bfa..6283fcb 100644
--- a/arch/arm/mach-pxa/raumfeld.c
+++ b/arch/arm/mach-pxa/raumfeld.c
@@ -505,6 +505,7 @@ static struct w1_gpio_platform_data w1_gpio_platform_data =
{
.pin= GPIO_ONE_WIRE,
.is_open_drain = 0,
.enable_external_pullup = w1_enable_external_pullup,
+ .ext_pullup_enable_pin = -EINVAL,
};
struct platform_device raumfeld_w1_gpio_device = {
--
1.8.1.5
--
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/
Re: [PATCH] ARM: w1-gpio: fix erroneous gpio requests
On Wed, Mar 13, 2013 at 04:24:20AM +0800, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> On Mar 13, 2013, at 4:20 AM, Greg Kroah-Hartman
> wrote:
> > On Tue, Mar 12, 2013 at 08:21:34PM +0100, Johan Hovold wrote:
> >> Fix regression introduced by commit d2323cf773 ("onewire: w1-gpio: add
> >> ext_pullup_enable pin in platform data") which added a gpio entry to the
> >> platform data, but did not add the required initialisers to the board
> >> files using it. Consequently, the driver would request gpio 0 at probe,
> >> which could break other uses of the corresponding pin.
> >>
> >> On AT91 requesting gpio 0 changes the pin muxing for PIOA0, which, for
> >> instance, breaks SPI0 on at91sam9g20.
>
> not only on AT91, 0 is a valid gpio
AT91 (and 9g20) was just an example of what the implications could be
like.
I discovered the change after having debugged broken MMC on a custom
at91sam9g45 board.
Johan
--
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/
[PATCH] ARM: at91/avr32/atmel-mci: fix DMA-channel leak on module unload
Fix regression introduced by commit 796211b7953 ("mmc: atmel-mci: add
pdc support and runtime capabilities detection") which removed the need
for CONFIG_MMC_ATMELMCI_DMA but kept the Kconfig-entry as well as the
compile guards around dma_release_channel() in remove(). Consequently,
DMA is always enabled (if supported), but the DMA-channel is not
released on module unload unless the DMA-config option is selected.
Remove the no longer used CONFIG_MMC_ATMELMCI_DMA option completely.
Cc: stable
Signed-off-by: Johan Hovold
---
arch/arm/configs/at91sam9g45_defconfig | 1 -
arch/avr32/configs/favr-32_defconfig | 1 -
arch/avr32/configs/merisc_defconfig| 1 -
drivers/mmc/host/Kconfig | 10 --
drivers/mmc/host/atmel-mci.c | 2 --
5 files changed, 15 deletions(-)
diff --git a/arch/arm/configs/at91sam9g45_defconfig
b/arch/arm/configs/at91sam9g45_defconfig
index 606d48f..8aab786 100644
--- a/arch/arm/configs/at91sam9g45_defconfig
+++ b/arch/arm/configs/at91sam9g45_defconfig
@@ -173,7 +173,6 @@ CONFIG_MMC=y
# CONFIG_MMC_BLOCK_BOUNCE is not set
CONFIG_SDIO_UART=m
CONFIG_MMC_ATMELMCI=y
-CONFIG_MMC_ATMELMCI_DMA=y
CONFIG_LEDS_ATMEL_PWM=y
CONFIG_LEDS_GPIO=y
CONFIG_LEDS_TRIGGER_TIMER=y
diff --git a/arch/avr32/configs/favr-32_defconfig
b/arch/avr32/configs/favr-32_defconfig
index 0421498..9791820 100644
--- a/arch/avr32/configs/favr-32_defconfig
+++ b/arch/avr32/configs/favr-32_defconfig
@@ -122,7 +122,6 @@ CONFIG_USB_G_SERIAL=m
CONFIG_USB_CDC_COMPOSITE=m
CONFIG_MMC=y
CONFIG_MMC_ATMELMCI=y
-CONFIG_MMC_ATMELMCI_DMA=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_ATMEL_PWM=m
diff --git a/arch/avr32/configs/merisc_defconfig
b/arch/avr32/configs/merisc_defconfig
index 3befab9..65de443 100644
--- a/arch/avr32/configs/merisc_defconfig
+++ b/arch/avr32/configs/merisc_defconfig
@@ -102,7 +102,6 @@ CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_LOGO=y
CONFIG_MMC=y
CONFIG_MMC_ATMELMCI=y
-CONFIG_MMC_ATMELMCI_DMA=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_ATMEL_PWM=y
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 8d13c65..009dabd 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -292,16 +292,6 @@ config MMC_ATMELMCI
If unsure, say N.
-config MMC_ATMELMCI_DMA
- bool "Atmel MCI DMA support"
- depends on MMC_ATMELMCI && (AVR32 || ARCH_AT91SAM9G45) && DMA_ENGINE
- help
- Say Y here to have the Atmel MCI driver use a DMA engine to
- do data transfers and thus increase the throughput and
- reduce the CPU utilization.
-
- If unsure, say N.
-
config MMC_MSM
tristate "Qualcomm SDCC Controller Support"
depends on MMC && ARCH_MSM
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 722af1d..10f8b73 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -2487,10 +2487,8 @@ static int __exit atmci_remove(struct platform_device
*pdev)
atmci_readl(host, ATMCI_SR);
clk_disable(host->mck);
-#ifdef CONFIG_MMC_ATMELMCI_DMA
if (host->dma.chan)
dma_release_channel(host->dma.chan);
-#endif
free_irq(platform_get_irq(pdev, 0), host);
iounmap(host->regs);
--
1.8.1.5
--
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/
Re: [PATCH v3 3/6] TTY: fix DTR being raised on hang up
On Wed, Mar 13, 2013 at 03:43:43PM -0400, Peter Hurley wrote:
> On Thu, 2013-03-07 at 15:55 +0100, Johan Hovold wrote:
> > Make sure to check ASYNC_INITIALISED before raising DTR when waking up
> > from blocked open in tty_port_block_til_ready.
> >
> > Currently DTR could get raised at hang up as a blocked process would
> > raise DTR unconditionally before checking for hang up and returning.
> >
> > Signed-off-by: Johan Hovold
> > ---
> > drivers/tty/tty_port.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> > index 3de5918..52f1066 100644
> > --- a/drivers/tty/tty_port.c
> > +++ b/drivers/tty/tty_port.c
> > @@ -355,7 +355,7 @@ int tty_port_block_til_ready(struct tty_port *port,
> >
> > while (1) {
> > /* Indicate we are open */
> > - if (tty->termios.c_cflag & CBAUD)
> > + if (C_BAUD(tty) && test_bit(ASYNCB_INITIALIZED, &port->flags))
> > tty_port_raise_dtr_rts(port);
> >
> > prepare_to_wait(&port->open_wait, &wait, TASK_INTERRUPTIBLE);
>
> This is ok, but there are 6 other *_block_til_ready() functions:
Yes, but that's not really a comment on this patch, is it?
The purpose of this series is to fix the tty-port implementation, and
I've only touched individual drivers when I had to in order not to break
anything due to changed assumptions.
There's a ton of buggy and odd behaviour to be found once you start
turning the stones. Drivers like the ones below really ought to be
using tty ports and it's helpers.
Thanks,
Johan
> This one doesn't use DTR/RTS so can be ignored:
> ./drivers/staging/sb105x/sb_pci_mp.c:static int mp_block_til_ready(struct
> file *filp, struct sb_uart_state *state)
>
> This one is so scary you should probably leave it alone:
> ./drivers/tty/serial/crisv10.c:block_til_ready(struct tty_struct *tty, struct
> file * filp,
>
> These will need the same change (although be careful: some use
> ASYNC_INITIALIZED rather than ASYNCB_INITIALIZED).
> ./drivers/tty/synclinkmp.c:static int block_til_ready(struct tty_struct *tty,
> struct file *filp,
> ./drivers/tty/synclink_gt.c:static int block_til_ready(struct tty_struct
> *tty, struct file *filp,
> ./drivers/tty/synclink.c:static int block_til_ready(struct tty_struct *tty,
> struct file * filp,
> ./net/irda/ircomm/ircomm_tty.c:static int ircomm_tty_block_til_ready(struct
> ircomm_tty_cb *self,
--
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/
Re: [PATCH v3 0/6] TTY: port hangup and close fixes
On Wed, Mar 13, 2013 at 03:50:32PM -0400, Peter Hurley wrote: > On Thu, 2013-03-07 at 15:55 +0100, Johan Hovold wrote: > > These patches against 3.9-rc1 fix a few issues with tty-port hangup and > > close. > > > > The first and fifth patch are essentially clean ups. > > > > The second and third patch fix the fact that DTR could get raised > > (rather than dropped) at hangup if there are blocked opens. [ Note that > > the second patch has been separated into its own patch and that the > > third patch is new in v3 of this series. ] > > > > The fourth patch makes sure DTR is dropped also on hangup and that DTR > > is only dropped for initialised ports (where it could have been raised > > in the first place). > > > > The sixth and final patch, makes sure no tty callbacks are made from > > tty_port_close_start when the port has not been initialised (successfully > > opened). This was previously only done for wait_until_sent but there's > > no reason to call flush_buffer or to honour port drain delay either. > > The latter could cause a failed open to stall for up to two seconds. > > > > As a side-effect, these patches also fix an issue in USB-serial where we > > could get tty-port callbacks on an uninitialised port after having hung > > up and unregistered a device after disconnect. > > > > Johan > > > > > > v3: > > - amend series with fix of DTR sometimes being raised on hang-up > > - do not lower DTR on hangup if port tty is gone > > - make sure tty in call to shutdown is refcounted > > - use cflag-macros throughout > > Other than the comments for patch 3/6, this series looks good. Thanks > again for your work on this. As I mentioned in my reply to 3/6, fixing bugs in other drivers not using the tty-port implementation is all good but not the purpose of this series. Thanks, Johan -- 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/
Re: [PATCH v3 3/6] TTY: fix DTR being raised on hang up
On Fri, Mar 15, 2013 at 07:03:08AM -0400, Peter Hurley wrote:
> On Fri, 2013-03-15 at 10:24 +0100, Johan Hovold wrote:
> > On Wed, Mar 13, 2013 at 03:43:43PM -0400, Peter Hurley wrote:
> > > On Thu, 2013-03-07 at 15:55 +0100, Johan Hovold wrote:
> > > > Make sure to check ASYNC_INITIALISED before raising DTR when waking up
> > > > from blocked open in tty_port_block_til_ready.
> > > >
> > > > Currently DTR could get raised at hang up as a blocked process would
> > > > raise DTR unconditionally before checking for hang up and returning.
> > > >
> > > > Signed-off-by: Johan Hovold
> > > > ---
> > > > drivers/tty/tty_port.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> > > > index 3de5918..52f1066 100644
> > > > --- a/drivers/tty/tty_port.c
> > > > +++ b/drivers/tty/tty_port.c
> > > > @@ -355,7 +355,7 @@ int tty_port_block_til_ready(struct tty_port *port,
> > > >
> > > > while (1) {
> > > > /* Indicate we are open */
> > > > - if (tty->termios.c_cflag & CBAUD)
> > > > + if (C_BAUD(tty) && test_bit(ASYNCB_INITIALIZED,
> > > > &port->flags))
> > > > tty_port_raise_dtr_rts(port);
> > > >
> > > > prepare_to_wait(&port->open_wait, &wait,
> > > > TASK_INTERRUPTIBLE);
> > >
> > > This is ok, but there are 6 other *_block_til_ready() functions:
> ^^
> Comment on patch
I saw that, but just wanted to stress that those comments shouldn't
block the series.
> > Yes, but that's not really a comment on this patch, is it?
> >
> > The purpose of this series is to fix the tty-port implementation, and
> > I've only touched individual drivers when I had to in order not to break
> > anything due to changed assumptions.
> >
> > There's a ton of buggy and odd behaviour to be found once you start
> > turning the stones. Drivers like the ones below really ought to be
> > using tty ports and it's helpers.
>
> Sure, I understand.
>
> OTOH, tty_port and these drivers stem from the same ancestor and it's
> partly because of localized bug fixes like these that the drivers have
> buggy and odd behavior (because tty_port gets fixed and these do not).
Arguably, fixing the core isn't really a localised bug fix. Some of
those drivers you mentioned have custom open, close, hangup which are
quite different from the tty port implementation, and surely would have
a lot to gain from being ported to tty ports if someone could find the
time to do so.
> As you can verify from the changelogs of these drivers, it's traditional
> to continue to maintain the common aspects, despite the desire to
> abandon them.
Most entries I see have to do with changed interfaces.
> That said, I'm not the maintainer so feel free to disagree with my
> point-of-view.
You do have a point, and I will try to find the time for a follow-up
series fixing at least a few of those five-or-so custom block_til_ready
you pointed to.
Thanks,
Johan
--
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/
Re: [PATCH v3 0/6] TTY: port hangup and close fixes
On Fri, Mar 15, 2013 at 12:05:58PM -0700, Greg KH wrote: > On Thu, Mar 07, 2013 at 03:55:47PM +0100, Johan Hovold wrote: > > These patches against 3.9-rc1 fix a few issues with tty-port hangup and > > close. > > Are these for 3.9-final? I'd say it can wait for 3.10 as it fixes long-standing issues without any really severe consequences: DTR is being raised or dropped when it shouldn't, some possibly annoying delays when open fails, and the callbacks after disconnect that we get in usb-serial that we are already work around. Johan -- 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/
Re: Fix memory leak in sierra_release() (this time with signed-off-by)
On Wed, Oct 24, 2012 at 10:23:09AM -0400, Lennart Sorensen wrote: > I found a memory leak in sierra_release() (well sierra_probe() I guess) > that looses 8 bytes each time the driver releases a device. Good catch! I missed this one when I fixed a bunch of other memory leaks in the sierra with recent kernels: http://marc.info/?l=linux-usb&m=135100550421848&w=2 I'll rebase my patch on top of this one as your patch should be backported to all stable kernels, whereas mine is only required for v3.6 and later. > Signed-off-by: Len Sorensen Cc: Acked-by: Johan Hovold Thanks, Johan > diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c > index 01d882c..76ef95b 100644 > --- a/drivers/usb/serial/sierra.c > +++ b/drivers/usb/serial/sierra.c > @@ -959,6 +959,7 @@ static void sierra_release(struct usb_serial *serial) > continue; > kfree(portdata); > } > + kfree(serial->private); > } > > #ifdef CONFIG_PM -- 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/
Re: [PATCH 1/5] USB : serial : remove tty arg of handle_dcd_change.
On Mon, Sep 09, 2013 at 06:01:16PM +0200, Paul Chavent wrote:
> Do the same way as in serialcore.c for uart_handle_dcd_change. It
> removes duplicated code around the usb_serial_handle_dcd_change calls.
>
> Signed-off-by: Paul Chavent
> ---
> drivers/usb/serial/ch341.c | 7 ++-
> drivers/usb/serial/generic.c | 4 ++--
> drivers/usb/serial/pl2303.c | 7 +--
> include/linux/usb/serial.h | 1 -
> 4 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index c2a4171..51c3d3a 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -481,11 +481,8 @@ static void ch341_read_int_callback(struct urb *urb)
> spin_unlock_irqrestore(&priv->lock, flags);
>
> if ((priv->line_status ^ prev_line_status) & CH341_BIT_DCD) {
> - struct tty_struct *tty = tty_port_tty_get(&port->port);
> - if (tty)
> - usb_serial_handle_dcd_change(port, tty,
> - priv->line_status & CH341_BIT_DCD);
> - tty_kref_put(tty);
> + usb_serial_handle_dcd_change(port,
> + priv->line_status & CH341_BIT_DCD);
> }
>
> wake_up_interruptible(&port->port.delta_msr_wait);
> diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
> index 1f31e6b..33f1df1 100644
> --- a/drivers/usb/serial/generic.c
> +++ b/drivers/usb/serial/generic.c
> @@ -560,13 +560,13 @@ EXPORT_SYMBOL_GPL(usb_serial_handle_break);
> /**
> * usb_serial_handle_dcd_change - handle a change of carrier detect state
> * @port: usb_serial_port structure for the open port
> - * @tty: tty_struct structure for the port
> * @status: new carrier detect status, nonzero if active
> */
> void usb_serial_handle_dcd_change(struct usb_serial_port *usb_port,
> - struct tty_struct *tty, unsigned int status)
> + unsigned int status)
> {
> struct tty_port *port = &usb_port->port;
> + struct tty_struct *tty = port->tty;
No, this is not right. There's a reason tty_port_tty_get was used. You
cannot simply remove it (even if your next patch adds it back, but
without the NULL check).
I'm actually preparing a series of changes to the MSR handling and
considered doing something like this, but came to the conclusion that
keeping the current interface was preferred (e.g. the same reference
could be used to add handle CTS changes as well). I'm refactoring at a
different level instead.
I suggest keeping the current interface for a while still, and that you
add the tty_port_tty_get to your ftdi patch instead.
Johan
--
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/
Re: [PATCH 1/5] USB : serial : remove tty arg of handle_dcd_change.
On Mon, Sep 09, 2013 at 07:45:23PM +0200, Johan Hovold wrote:
> On Mon, Sep 09, 2013 at 06:01:16PM +0200, Paul Chavent wrote:
> > Do the same way as in serialcore.c for uart_handle_dcd_change. It
> > removes duplicated code around the usb_serial_handle_dcd_change calls.
> >
> > Signed-off-by: Paul Chavent
> > ---
> > drivers/usb/serial/ch341.c | 7 ++-
> > drivers/usb/serial/generic.c | 4 ++--
> > drivers/usb/serial/pl2303.c | 7 +--
> > include/linux/usb/serial.h | 1 -
> > 4 files changed, 5 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> > index c2a4171..51c3d3a 100644
> > --- a/drivers/usb/serial/ch341.c
> > +++ b/drivers/usb/serial/ch341.c
> > @@ -481,11 +481,8 @@ static void ch341_read_int_callback(struct urb *urb)
> > spin_unlock_irqrestore(&priv->lock, flags);
> >
> > if ((priv->line_status ^ prev_line_status) & CH341_BIT_DCD) {
> > - struct tty_struct *tty = tty_port_tty_get(&port->port);
> > - if (tty)
> > - usb_serial_handle_dcd_change(port, tty,
> > - priv->line_status & CH341_BIT_DCD);
> > - tty_kref_put(tty);
> > + usb_serial_handle_dcd_change(port,
> > + priv->line_status & CH341_BIT_DCD);
> > }
> >
> > wake_up_interruptible(&port->port.delta_msr_wait);
> > diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
> > index 1f31e6b..33f1df1 100644
> > --- a/drivers/usb/serial/generic.c
> > +++ b/drivers/usb/serial/generic.c
> > @@ -560,13 +560,13 @@ EXPORT_SYMBOL_GPL(usb_serial_handle_break);
> > /**
> > * usb_serial_handle_dcd_change - handle a change of carrier detect state
> > * @port: usb_serial_port structure for the open port
> > - * @tty: tty_struct structure for the port
> > * @status: new carrier detect status, nonzero if active
> > */
> > void usb_serial_handle_dcd_change(struct usb_serial_port *usb_port,
> > - struct tty_struct *tty, unsigned int status)
> > + unsigned int status)
> > {
> > struct tty_port *port = &usb_port->port;
> > + struct tty_struct *tty = port->tty;
>
> No, this is not right. There's a reason tty_port_tty_get was used. You
> cannot simply remove it (even if your next patch adds it back, but
> without the NULL check).
My bad, the NULL check was already there. But this still would have to
be done as one patch, although I prefer keeping the current interface
for now.
Johan
--
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/
Re: [PATCH 1/3] USB : serial : call handle_dcd_change in ftdi driver.
On Fri, Sep 13, 2013 at 05:35:11PM +0200, Paul Chavent wrote:
> Signed-off-by: Paul Chavent
> ---
> drivers/usb/serial/ftdi_sio.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index c45f9c0..df66495 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -1966,8 +1966,14 @@ static int ftdi_process_packet(struct usb_serial_port
> *port,
> port->icount.dsr++;
> if (diff_status & FTDI_RS0_RI)
> port->icount.rng++;
> - if (diff_status & FTDI_RS0_RLSD)
> + if (diff_status & FTDI_RS0_RLSD) {
> port->icount.dcd++;
> + struct tty_struct *tty = tty_port_tty_get(&port->port);
Please move the tty_struct definition to the top of the function (or at
least to the top of the block).
Thanks,
Johan
--
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/
Re: [PATCH 2/3] USB : serial : invoke dcd_change ldisc's handler.
On Fri, Sep 13, 2013 at 05:35:12PM +0200, Paul Chavent wrote:
> Signed-off-by: Paul Chavent
> ---
> Documentation/pps/pps.txt| 15 +++
> drivers/usb/serial/generic.c | 9 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/Documentation/pps/pps.txt b/Documentation/pps/pps.txt
> index d35dcdd..67b9a94 100644
> --- a/Documentation/pps/pps.txt
> +++ b/Documentation/pps/pps.txt
> @@ -66,6 +66,21 @@ In LinuxPPS the PPS sources are simply char devices
> usually mapped
> into files /dev/pps0, /dev/pps1, etc..
>
>
> +PPS with USB to serial devices
> +--
> +
> +It is possible to grab the PPS from an USB to serial device. However,
> +you should take into account the latencies and jitter introduced by
> +the USB stack. Users has reported clock instability around +-1ms when
> +synchronized with PPS through USB. This isn't suited for time server
> +synchronisation.
> +
> +If your device doesn't report PPS, you can check that the feature is
> +supported by its driver. Most of the time, you only need to add a call
> +to usb_serial_handle_dcd_change after checking the DCD status (see
> +ch341 and pl2303 examples).
> +
> +
> Coding example
> --
>
> diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
> index 1f31e6b..877d6e0 100644
> --- a/drivers/usb/serial/generic.c
> +++ b/drivers/usb/serial/generic.c
> @@ -568,6 +568,15 @@ void usb_serial_handle_dcd_change(struct usb_serial_port
> *usb_port,
> {
> struct tty_port *port = &usb_port->port;
>
> + if (tty) {
> + struct tty_ldisc *ld = tty_ldisc_ref(tty);
> + if (ld) {
> + if (ld->ops->dcd_change)
> + ld->ops->dcd_change(tty, status);
> + tty_ldisc_deref(ld);
> + }
> + }
> +
> dev_dbg(&usb_port->dev, "%s - status %d\n", __func__, status);
Please add your ldisc handling after the debug statement.
Thanks,
Johan
--
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/
Re: [PATCH 3/3] USB : serial : pl2303 wake up after dcd status check.
On Fri, Sep 13, 2013 at 05:35:13PM +0200, Paul Chavent wrote:
> Seems to be done this way in other drivers (ch341, 8250, ...).
> And get tty reference only if dcd_change need to be called.
This is fine. I have a patch here doing the same two changes as part of
a larger clean-up of the pl2303 interrupt handling (which in turn is
part of the MSR-refactoring I mentioned). I could rebase on top of this,
unless you care to wait another week. :)
Thanks,
Johan
> Signed-off-by: Paul Chavent
> ---
> drivers/usb/serial/pl2303.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index e7a84f0..8b81188 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -854,15 +854,16 @@ static void pl2303_update_line_status(struct
> usb_serial_port *port,
> spin_unlock_irqrestore(&priv->lock, flags);
> if (priv->line_status & UART_BREAK_ERROR)
> usb_serial_handle_break(port);
> - wake_up_interruptible(&port->port.delta_msr_wait);
>
> - tty = tty_port_tty_get(&port->port);
> - if (!tty)
> - return;
> - if ((priv->line_status ^ prev_line_status) & UART_DCD)
> - usb_serial_handle_dcd_change(port, tty,
> - priv->line_status & UART_DCD);
> - tty_kref_put(tty);
> + if ((priv->line_status ^ prev_line_status) & UART_DCD) {
> + tty = tty_port_tty_get(&port->port);
> + if (tty)
> + usb_serial_handle_dcd_change(port, tty,
> + priv->line_status & UART_DCD);
> + tty_kref_put(tty);
> + }
> +
> + wake_up_interruptible(&port->port.delta_msr_wait);
> }
>
> static void pl2303_read_int_callback(struct urb *urb)
> --
> 1.7.12.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/
[PATCH] driver core: prevent deferred probe with platform_driver_probe
Prevent drivers relying on platform_driver_probe from requesting
deferred probing in order to avoid further futile probe attempts (either
the driver has been unregistered or its probe function has been set to
platform_drv_probe_fail when probing is retried).
Note that several platform drivers currently return subsystem errors
from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
request fails).
Signed-off-by: Johan Hovold
---
drivers/base/platform.c | 17 +
include/linux/platform_device.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..47051cd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -488,6 +488,11 @@ static int platform_drv_probe(struct device *_dev)
if (ret && ACPI_HANDLE(_dev))
acpi_dev_pm_detach(_dev, true);
+ if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
+ dev_warn(_dev, "probe deferral not supported\n");
+ ret = -ENXIO;
+ }
+
return ret;
}
@@ -553,8 +558,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
/**
* platform_driver_probe - register driver for non-hotpluggable device
* @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section,
- * must not return -EPROBE_DEFER.
+ * @probe: the driver probe routine, probably from an __init section
*
* Use this instead of platform_driver_register() when you know the device
* is not hotpluggable and has already been registered, and you want to
@@ -565,8 +569,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
* into system-on-chip processors, where the controller devices have been
* configured as part of board setup.
*
- * This is incompatible with deferred probing so probe() must not
- * return -EPROBE_DEFER.
+ * Note that this is incompatible with deferred probing.
*
* Returns zero if the driver registered and bound to a device, else returns
* a negative error code and with the driver not registered.
@@ -576,6 +579,12 @@ int __init_or_module platform_driver_probe(struct
platform_driver *drv,
{
int retval, code;
+ /*
+* Prevent driver from requesting probe deferral to avoid further
+* futile probe attempts.
+*/
+ drv->prevent_deferred_probe = true;
+
/* make sure driver won't have bind/unbind attributes */
drv->driver.suppress_bind_attrs = true;
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index ce8e4ff..16f6654 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -178,6 +178,7 @@ struct platform_driver {
int (*resume)(struct platform_device *);
struct device_driver driver;
const struct platform_device_id *id_table;
+ bool prevent_deferred_probe;
};
#define to_platform_driver(drv)(container_of((drv), struct
platform_driver, \
--
1.8.3.2
--
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/
Re: [PATCH] driver core: prevent deferred probe with platform_driver_probe
On Mon, Sep 23, 2013 at 12:01:40PM +0100, Mark Brown wrote: > On Mon, Sep 23, 2013 at 10:48:47AM +0200, Johan Hovold wrote: > > Prevent drivers relying on platform_driver_probe from requesting > > deferred probing in order to avoid further futile probe attempts (either > > the driver has been unregistered or its probe function has been set to > > platform_drv_probe_fail when probing is retried). > > > > Note that several platform drivers currently return subsystem errors > > from probe and that these can include -EPROBE_DEFER (e.g. if a gpio > > request fails). > > This doesn't seem like the right end to address the problem from, it > seems like it would be better to move these drivers over to being normal > plaform drivers. Using module_platform_driver() means relying on init > ordering which is the sort of thing we're trying to get away from. I actually started out doing that, but it's getting a bit hard to audit which drivers could actually request probe deferral since gpio and later other subsystems started returning -EPROBE_DEFER. I found six by just grepping for gpio_request, but some of these calls can be made in helper functions (e.g. mmc_gpio_request_cd even though that one was easy to find). Having a warning printed by platform_drv_probe if a platform driver inadvertently requests probe deferral could be useful to catch any mistakes even if we start moving probe functions out of __init. I'll fix up the six drivers I found meanwhile. Johan -- 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/
[PATCH 2/7] mmc: mvsdio: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if the mmc_gpio_request_cd fails.
Cc: Nicolas Pitre
Cc: Chris Ball
Signed-off-by: Johan Hovold
---
drivers/mmc/host/mvsdio.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 06c5b0b..deecee0 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -655,7 +655,7 @@ static const struct mmc_host_ops mvsd_ops = {
.enable_sdio_irq= mvsd_enable_sdio_irq,
};
-static void __init
+static void
mv_conf_mbus_windows(struct mvsd_host *host,
const struct mbus_dram_target_info *dram)
{
@@ -677,7 +677,7 @@ mv_conf_mbus_windows(struct mvsd_host *host,
}
}
-static int __init mvsd_probe(struct platform_device *pdev)
+static int mvsd_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct mmc_host *mmc = NULL;
@@ -819,7 +819,7 @@ out:
return ret;
}
-static int __exit mvsd_remove(struct platform_device *pdev)
+static int mvsd_remove(struct platform_device *pdev)
{
struct mmc_host *mmc = platform_get_drvdata(pdev);
@@ -872,7 +872,8 @@ static const struct of_device_id mvsdio_dt_ids[] = {
MODULE_DEVICE_TABLE(of, mvsdio_dt_ids);
static struct platform_driver mvsd_driver = {
- .remove = __exit_p(mvsd_remove),
+ .probe = mvsd_probe,
+ .remove = mvsd_remove,
.suspend= mvsd_suspend,
.resume = mvsd_resume,
.driver = {
@@ -881,7 +882,7 @@ static struct platform_driver mvsd_driver = {
},
};
-module_platform_driver_probe(mvsd_driver, mvsd_probe);
+module_platform_driver(mvsd_driver);
/* maximum card clock frequency (default 50MHz) */
module_param(maxfreq, int, 0);
--
1.8.3.2
--
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/
[PATCH 7/7] backlight: atmel-pwm-bl: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: Richard Purdie
Cc: Jingoo Han
Cc: Jean-Christophe Plagniol-Villard
Signed-off-by: Johan Hovold
---
drivers/video/backlight/atmel-pwm-bl.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c
b/drivers/video/backlight/atmel-pwm-bl.c
index 0393d82..f7447f7 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -118,7 +118,7 @@ static const struct backlight_ops atmel_pwm_bl_ops = {
.update_status = atmel_pwm_bl_set_intensity,
};
-static int __init atmel_pwm_bl_probe(struct platform_device *pdev)
+static int atmel_pwm_bl_probe(struct platform_device *pdev)
{
struct backlight_properties props;
const struct atmel_pwm_bl_platform_data *pdata;
@@ -202,7 +202,7 @@ err_free_mem:
return retval;
}
-static int __exit atmel_pwm_bl_remove(struct platform_device *pdev)
+static int atmel_pwm_bl_remove(struct platform_device *pdev)
{
struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
@@ -220,10 +220,11 @@ static struct platform_driver atmel_pwm_bl_driver = {
.name = "atmel-pwm-bl",
},
/* REVISIT add suspend() and resume() */
- .remove = __exit_p(atmel_pwm_bl_remove),
+ .probe = atmel_pwm_bl_probe,
+ .remove = atmel_pwm_bl_remove,
};
-module_platform_driver_probe(atmel_pwm_bl_driver, atmel_pwm_bl_probe);
+module_platform_driver(atmel_pwm_bl_driver);
MODULE_AUTHOR("Hans-Christian egtvedt ");
MODULE_DESCRIPTION("Atmel PWM backlight driver");
--
1.8.3.2
--
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/
[PATCH 5/7] usb: gadget: pxa25x_udc: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: Eric Miao
Cc: Russell King
Cc: Haojian Zhuang
Cc: Felipe Balbi
Signed-off-by: Johan Hovold
---
drivers/usb/gadget/pxa25x_udc.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
index cc92074..0ac6064 100644
--- a/drivers/usb/gadget/pxa25x_udc.c
+++ b/drivers/usb/gadget/pxa25x_udc.c
@@ -2054,7 +2054,7 @@ static struct pxa25x_udc memory = {
/*
* probe - binds to the platform device
*/
-static int __init pxa25x_udc_probe(struct platform_device *pdev)
+static int pxa25x_udc_probe(struct platform_device *pdev)
{
struct pxa25x_udc *dev = &memory;
int retval, irq;
@@ -2203,7 +2203,7 @@ static void pxa25x_udc_shutdown(struct platform_device
*_dev)
pullup_off();
}
-static int __exit pxa25x_udc_remove(struct platform_device *pdev)
+static int pxa25x_udc_remove(struct platform_device *pdev)
{
struct pxa25x_udc *dev = platform_get_drvdata(pdev);
@@ -2294,7 +2294,8 @@ static int pxa25x_udc_resume(struct platform_device *dev)
static struct platform_driver udc_driver = {
.shutdown = pxa25x_udc_shutdown,
- .remove = __exit_p(pxa25x_udc_remove),
+ .probe = pxa25x_udc_probe,
+ .remove = pxa25x_udc_remove,
.suspend= pxa25x_udc_suspend,
.resume = pxa25x_udc_resume,
.driver = {
@@ -2303,7 +2304,7 @@ static struct platform_driver udc_driver = {
},
};
-module_platform_driver_probe(udc_driver, pxa25x_udc_probe);
+module_platform_driver(udc_driver);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR("Frank Becker, Robert Schwebel, David Brownell");
--
1.8.3.2
--
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/
[PATCH 3/7] mtd: atmel_nand: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: David Woodhouse
Cc: Josh Wu
Signed-off-by: Johan Hovold
---
drivers/mtd/nand/atmel_nand.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 060feea..bd1ce7d 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1139,7 +1139,7 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
return 0;
}
-static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
+static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
struct atmel_nand_host *host)
{
struct mtd_info *mtd = &host->mtd;
@@ -1548,7 +1548,7 @@ static int atmel_of_init_port(struct atmel_nand_host
*host,
}
#endif
-static int __init atmel_hw_nand_init_params(struct platform_device *pdev,
+static int atmel_hw_nand_init_params(struct platform_device *pdev,
struct atmel_nand_host *host)
{
struct mtd_info *mtd = &host->mtd;
@@ -1987,7 +1987,7 @@ static struct platform_driver atmel_nand_nfc_driver;
/*
* Probe for the NAND device.
*/
-static int __init atmel_nand_probe(struct platform_device *pdev)
+static int atmel_nand_probe(struct platform_device *pdev)
{
struct atmel_nand_host *host;
struct mtd_info *mtd;
@@ -2184,7 +2184,7 @@ err_nand_ioremap:
/*
* Remove a NAND device.
*/
-static int __exit atmel_nand_remove(struct platform_device *pdev)
+static int atmel_nand_remove(struct platform_device *pdev)
{
struct atmel_nand_host *host = platform_get_drvdata(pdev);
struct mtd_info *mtd = &host->mtd;
@@ -2270,7 +2270,8 @@ static struct platform_driver atmel_nand_nfc_driver = {
};
static struct platform_driver atmel_nand_driver = {
- .remove = __exit_p(atmel_nand_remove),
+ .probe = atmel_nand_probe,
+ .remove = atmel_nand_remove,
.driver = {
.name = "atmel_nand",
.owner = THIS_MODULE,
@@ -2278,7 +2279,7 @@ static struct platform_driver atmel_nand_driver = {
},
};
-module_platform_driver_probe(atmel_nand_driver, atmel_nand_probe);
+module_platform_driver(atmel_nand_driver);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Rick Bronson");
--
1.8.3.2
--
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/
[PATCH 4/7] pcmcia: at91_cf: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.
Cc: Jean-Christophe PLAGNIOL-VILLARD
Cc: Nicolas Ferre
Signed-off-by: Johan Hovold
---
drivers/pcmcia/at91_cf.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/pcmcia/at91_cf.c b/drivers/pcmcia/at91_cf.c
index b8f5acf..de24232 100644
--- a/drivers/pcmcia/at91_cf.c
+++ b/drivers/pcmcia/at91_cf.c
@@ -245,7 +245,7 @@ static int at91_cf_dt_init(struct platform_device *pdev)
}
#endif
-static int __init at91_cf_probe(struct platform_device *pdev)
+static int at91_cf_probe(struct platform_device *pdev)
{
struct at91_cf_socket *cf;
struct at91_cf_data *board = pdev->dev.platform_data;
@@ -354,7 +354,7 @@ fail0a:
return status;
}
-static int __exit at91_cf_remove(struct platform_device *pdev)
+static int at91_cf_remove(struct platform_device *pdev)
{
struct at91_cf_socket *cf = platform_get_drvdata(pdev);
@@ -404,14 +404,13 @@ static struct platform_driver at91_cf_driver = {
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(at91_cf_dt_ids),
},
- .remove = __exit_p(at91_cf_remove),
+ .probe = at91_cf_probe,
+ .remove = at91_cf_remove,
.suspend= at91_cf_suspend,
.resume = at91_cf_resume,
};
-/*--*/
-
-module_platform_driver_probe(at91_cf_driver, at91_cf_probe);
+module_platform_driver(at91_cf_driver);
MODULE_DESCRIPTION("AT91 Compact Flash Driver");
MODULE_AUTHOR("David Brownell");
--
1.8.3.2
--
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/
[PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
Deferred probing cannot be used with platform_driver_probe as by the
time probing is retried either the driver has been unregistered or its
probe function has been set to platform_drv_probe_fail.
With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
the gpio subsystem started returning -EPROBE_DEFER, which in turn
several platform drivers using platform_driver_probe return to driver
core. Other subsystems (e.g. regulator) has since started doing the
same.
The first patch in this series prevents platform drivers using
platform_driver_probe from requesting probe deferral while warning that
it is not supported.
The remaining patches move six platform-driver probe functions that rely
on gpio_request out of __init. There are likely other probe functions
that might return -EPROBE_DEFER and should be moved out of __init as
well.
Note that the mvsdio, at91_cf and pxa25x_udc patches are completely
untested.
Johan
Johan Hovold (7):
driver core: prevent deferred probe with platform_driver_probe
mmc: mvsdio: fix deferred probe from __init
mtd: atmel_nand: fix deferred probe from __init
pcmcia: at91_cf: fix deferred probe from __init
usb: gadget: pxa25x_udc: fix deferred probe from __init
usb: phy: gpio-vbus: fix deferred probe from __init
backlight: atmel-pwm-bl: fix deferred probe from __init
drivers/base/platform.c| 17 +
drivers/mmc/host/mvsdio.c | 11 ++-
drivers/mtd/nand/atmel_nand.c | 13 +++--
drivers/pcmcia/at91_cf.c | 11 +--
drivers/usb/gadget/pxa25x_udc.c| 9 +
drivers/usb/phy/phy-gpio-vbus-usb.c| 11 +--
drivers/video/backlight/atmel-pwm-bl.c | 9 +
include/linux/platform_device.h| 1 +
8 files changed, 47 insertions(+), 35 deletions(-)
--
1.8.3.2
--
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/
[PATCH 1/7] driver core: prevent deferred probe with platform_driver_probe
Prevent drivers relying on platform_driver_probe from requesting
deferred probing in order to avoid further futile probe attempts (either
the driver has been unregistered or its probe function has been set to
platform_drv_probe_fail when probing is retried).
Note that several platform drivers currently return subsystem errors
from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
request fails).
Add a warning to platform_drv_probe that can be used to catch drivers
that inadvertently request probe deferral while using
platform_driver_probe.
Signed-off-by: Johan Hovold
---
drivers/base/platform.c | 17 +
include/linux/platform_device.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..47051cd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -488,6 +488,11 @@ static int platform_drv_probe(struct device *_dev)
if (ret && ACPI_HANDLE(_dev))
acpi_dev_pm_detach(_dev, true);
+ if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
+ dev_warn(_dev, "probe deferral not supported\n");
+ ret = -ENXIO;
+ }
+
return ret;
}
@@ -553,8 +558,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
/**
* platform_driver_probe - register driver for non-hotpluggable device
* @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section,
- * must not return -EPROBE_DEFER.
+ * @probe: the driver probe routine, probably from an __init section
*
* Use this instead of platform_driver_register() when you know the device
* is not hotpluggable and has already been registered, and you want to
@@ -565,8 +569,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
* into system-on-chip processors, where the controller devices have been
* configured as part of board setup.
*
- * This is incompatible with deferred probing so probe() must not
- * return -EPROBE_DEFER.
+ * Note that this is incompatible with deferred probing.
*
* Returns zero if the driver registered and bound to a device, else returns
* a negative error code and with the driver not registered.
@@ -576,6 +579,12 @@ int __init_or_module platform_driver_probe(struct
platform_driver *drv,
{
int retval, code;
+ /*
+* Prevent driver from requesting probe deferral to avoid further
+* futile probe attempts.
+*/
+ drv->prevent_deferred_probe = true;
+
/* make sure driver won't have bind/unbind attributes */
drv->driver.suppress_bind_attrs = true;
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index ce8e4ff..16f6654 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -178,6 +178,7 @@ struct platform_driver {
int (*resume)(struct platform_device *);
struct device_driver driver;
const struct platform_device_id *id_table;
+ bool prevent_deferred_probe;
};
#define to_platform_driver(drv)(container_of((drv), struct
platform_driver, \
--
1.8.3.2
--
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/
[PATCH 6/7] usb: phy: gpio-vbus: fix deferred probe from __init
Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.
Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
and 04bf3011 ("regulator: Support driver probe deferral") this driver
might return -EPROBE_DEFER if a gpio_request or regulator_get fails.
Cc: Felipe Balbi
Signed-off-by: Johan Hovold
---
drivers/usb/phy/phy-gpio-vbus-usb.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c
b/drivers/usb/phy/phy-gpio-vbus-usb.c
index b2f29c9..02799a5 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -241,7 +241,7 @@ static int gpio_vbus_set_suspend(struct usb_phy *phy, int
suspend)
/* platform driver interface */
-static int __init gpio_vbus_probe(struct platform_device *pdev)
+static int gpio_vbus_probe(struct platform_device *pdev)
{
struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
struct gpio_vbus_data *gpio_vbus;
@@ -349,7 +349,7 @@ err_gpio:
return err;
}
-static int __exit gpio_vbus_remove(struct platform_device *pdev)
+static int gpio_vbus_remove(struct platform_device *pdev)
{
struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
@@ -398,8 +398,6 @@ static const struct dev_pm_ops gpio_vbus_dev_pm_ops = {
};
#endif
-/* NOTE: the gpio-vbus device may *NOT* be hotplugged */
-
MODULE_ALIAS("platform:gpio-vbus");
static struct platform_driver gpio_vbus_driver = {
@@ -410,10 +408,11 @@ static struct platform_driver gpio_vbus_driver = {
.pm = &gpio_vbus_dev_pm_ops,
#endif
},
- .remove = __exit_p(gpio_vbus_remove),
+ .probe = gpio_vbus_probe,
+ .remove = gpio_vbus_remove,
};
-module_platform_driver_probe(gpio_vbus_driver, gpio_vbus_probe);
+module_platform_driver(gpio_vbus_driver);
MODULE_DESCRIPTION("simple GPIO controlled OTG transceiver driver");
MODULE_AUTHOR("Philipp Zabel");
--
1.8.3.2
--
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/
Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
On Mon, Sep 23, 2013 at 04:50:29PM +0200, Sascha Hauer wrote:
> On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> > Deferred probing cannot be used with platform_driver_probe as by the
> > time probing is retried either the driver has been unregistered or its
> > probe function has been set to platform_drv_probe_fail.
> >
> > With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> > the gpio subsystem started returning -EPROBE_DEFER, which in turn
> > several platform drivers using platform_driver_probe return to driver
> > core. Other subsystems (e.g. regulator) has since started doing the
> > same.
> >
> > The first patch in this series prevents platform drivers using
> > platform_driver_probe from requesting probe deferral while warning that
> > it is not supported.
> >
> > The remaining patches move six platform-driver probe functions that rely
> > on gpio_request out of __init. There are likely other probe functions
> > that might return -EPROBE_DEFER and should be moved out of __init as
> > well.
>
> As usually when I read this I wonder why platform_driver_probe exists
> anyway. The only advantage I can think off is that the probe functions
> are in __init and thus can be disposed of later. Now you remove the
> __init annotations from these probe functions. Wouldn't it be better to
> convert the drivers to regular platform_driver_register instead?
Perhaps that paragraph was a bit unclear: I move them out of __init
_and_ use platform_driver_register instead of platform_driver_probe as
well.
Johan
--
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/
[PATCH] ARM: at91/ide: remove unsused at91-ide Kconfig entry
Commit cf844751fb25e ("ARM: at91: drop ide driver in favor of the pata
one") removed the at91-ide driver but did not remove the Kconfig entry.
Signed-off-by: Johan Hovold
---
drivers/ide/Kconfig | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig
index 5a26584..bfec4e6 100644
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -702,11 +702,6 @@ config BLK_DEV_IDE_TX4939
depends on SOC_TX4939
select BLK_DEV_IDEDMA_SFF
-config BLK_DEV_IDE_AT91
- tristate "Atmel AT91 (SAM9, CAP9, AT572D940HF) IDE support"
- depends on ARM && ARCH_AT91 && !ARCH_AT91RM9200 && !ARCH_AT91X40
- select IDE_TIMINGS
-
config BLK_DEV_IDE_ICSIDE
tristate "ICS IDE interface support"
depends on ARM && ARCH_ACORN
--
1.8.0
--
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/
Re: kmemleak report on isp1763 and sierra MC8705
On Wed, Nov 14, 2012 at 12:12:01PM -0500, Richard Retanubun wrote:
> On 10/11/12 09:30 AM, Johan Hovold wrote:
> Hi Johan,
>
> > There was a reference-count fix for the probe error path that went in to
> > v3.5. Haven't read all the details on how you trigger your leak, but at
> > the face of it, it could be related.
> >
> > Have a look at 0658a3366db7e27fa ("usb: use usb_serial_put in
> > usb_serial_probe errors). If related, you should be seeing "Ignoring
> > blacklisted interface #n" messages when you enable debug (e.g. #define
> > DEBUG) in the sierra driver.
>
> That was it! Thanks so much for the research.
> I can apply it cleanly to 3.0.22 and see usb_release_dev() being
> called and thus no more kmemleak.
>
> >
> > Greg, it seems to me that the fix referred to above should be backported
> > to the earlier stable trees either way.
> I would vote "yes" for this also.
>
> While my setup circumstances may be a corner case, (modem kept
> resetting to re-establish PPP connection) it was leaking 1192 bytes
> per occurrence.
The leak affects every failed probe, for example due to blacklisted
interfaces which is quite common, so commit 0658a3366db7 ("usb: use
usb_serial_put in usb_serial_probe errors) should be backported to the
<= 3.4 stable trees.
Thanks for reporting,
Johan
--
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/
[PATCH] USB: mos7840: remove unused variable
Fix warning about unused variable introduced by commit e681b66f2e19fa
("USB: mos7840: remove invalid disconnect handling") upstream.
A subsequent fix which removed the disconnect function got rid of the
warning but that one was only backported to v3.6.
Reported-by: Jiri Slaby
Cc: stable
Signed-off-by: Johan Hovold
---
This patch against v3.4 also applies to v3.0 (and v3.2).
Thanks,
Johan
drivers/usb/serial/mos7840.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 0179d34..c854235 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -2581,7 +2581,6 @@ error:
static void mos7840_disconnect(struct usb_serial *serial)
{
int i;
- unsigned long flags;
struct moschip_port *mos7840_port;
dbg("%s", " disconnect :entering..");
--
1.7.12
--
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/
Re: [PATCH usb-linus] USB: keyspan: fix typo causing GPF on open
On Sat, Nov 10, 2012 at 10:13 AM, Bjørn Mork wrote:
> Commit f79b2d0f (USB: keyspan: fix NULL-pointer dereferences and
> memory leaks) had a small typo which made the driver use wrong
> offsets when mapping serial port private data. This results in
> in a GPF when the port is opened.
>
> Reported-by: Richard
> Cc:
> Cc: Johan Hovold
> Signed-off-by: Bjørn Mork
Acked-by: Johan Hovold
> ---
> Hello Richard,
>
> I wonder if you are able to test and verify this? I do not guarantee
> that there aren't other issues around, but this small typo looked like
> an obvious killer...
Good catch, Bjørn.
Greg, this one needs to be backported to v3.6.x.
Note that the usb_get_serial_data call below was simply redundant,
so that part is really a clean up rather than part of the fix.
Thanks,
Johan
> Bjørn
>
> drivers/usb/serial/keyspan.c |3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
> index 7179b0c..cff8dd5 100644
> --- a/drivers/usb/serial/keyspan.c
> +++ b/drivers/usb/serial/keyspan.c
> @@ -2430,7 +2430,7 @@ static void keyspan_release(struct usb_serial *serial)
> static int keyspan_port_probe(struct usb_serial_port *port)
> {
> struct usb_serial *serial = port->serial;
> - struct keyspan_port_private *s_priv;
> + struct keyspan_serial_private *s_priv;
> struct keyspan_port_private *p_priv;
> const struct keyspan_device_details *d_details;
> struct callbacks *cback;
> @@ -2445,7 +2445,6 @@ static int keyspan_port_probe(struct usb_serial_port
> *port)
> if (!p_priv)
> return -ENOMEM;
>
> - s_priv = usb_get_serial_data(port->serial);
> p_priv->device_details = d_details;
>
> /* Setup values for the various callback routines */
> --
> 1.7.10.4
>
--
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/
Re: kmemleak report on isp1763 and sierra MC8705
On Fri, Nov 9, 2012 at 11:14 PM, Richard Retanubun
wrote:
> On 29/10/12 06:14 PM, Alan Stern wrote:
>>
>> On Mon, 29 Oct 2012, Richard Retanubun wrote:
>>>
>>> Focusing down on one of the dumps:
>>>
>>> unreferenced object 0xd3849740 (size 8):
>>> comm "khubd", pid 1026, jiffies 232553037 (age 506.597s)
>>> hex dump (first 8 bytes):
>>> 4d 43 38 37 30 35 00 00 MC8705..
>>> backtrace:
>>> [] usb_cache_string+0x74/0xac [usbcore]
>>> [] usb_enumerate_device+0x44/0xf8 [usbcore]
>>> [] usb_new_device+0x3c/0x13c [usbcore]
>>> [] hub_thread+0xc8c/0x1544 [usbcore]
>>> [] kthread+0x7c/0x80
>>> [] kernel_thread+0x4c/0x68
>>>
>>> I have a small question. How does the memory kmalloc-ed() in
>>> usb_cache_string is supposed to be released?
>>> (during usb_serial_disconnect()?)
>>
>>
>> It doesn't get released during usb_serial_disconnect(). It gets
>> released during usb_release_dev() in drivers/usb/core/usb.c.
>>
>>> Is the sierra driver is supposed to participate
>>> in the tear down process (in sierra_release() maybe) and not doing
>>> something that is expected?
>>
>>
>> Probably not.
>>
>>> I am still missing the link between the actions done by the hub_thread()
>>> for the caching the stings
>>> and the sierra driver code.
>>
>>
>> They aren't all that closely related.
>>
>> usb_release_dev() won't be called until all references to the USB
>> device have been dropped. Maybe there's an extra reference hanging
>> around.
>>
>> Alan Stern
>>
> Thanks a lot for the hint Alan.
>
> I added a dev_dbg print in usb_release_dev() and saw that in the builds
> where there is a leak, this was simply never called!
> the last line printed in a trace with all dev_dbg on is this
> "usb_disable_device nuking all URBs"
> When the sierra modem is unplugged, the cleanup sequence never calls
> usb_release_dev() (on PL2303 it always calls usb_release_dev()
>
> This is the current state of versions from linux-stable
>
> 3.0.y (3.0.51) - Have the issue
> 3.2.y (3.2.33) - Have the issue
> 3.4.y (3.4.18) - Have the issue
> 3.5.y (3.5.7) - Does not have the issue (but leaks because the portdata
> patches is not backported yet)
> 3.6.y (3.6.6) - Does not have the issue
>
> So a diff between 3.4.y and 3.5.y ought to narrow it down further.
>
> I am posting just in case someone recalls a particular patch I should be
> trying out first...
There was a reference-count fix for the probe error path that went in to
v3.5. Haven't read all the details on how you trigger your leak, but at
the face of it, it could be related.
Have a look at 0658a3366db7e27fa ("usb: use usb_serial_put in
usb_serial_probe errors). If related, you should be seeing "Ignoring
blacklisted interface #n" messages when you enable debug (e.g. #define
DEBUG) in the sierra driver.
Greg, it seems to me that the fix referred to above should be backported
to the earlier stable trees either way.
Thanks,
Johan
--
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/
Re: [RFC] raise the maximum number of usb-serial devices to 512
On Mon, Jun 03, 2013 at 07:49:59PM -0700, Greg KH wrote:
> On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
> > But, IMHO, a nicer approach would be to make the allocation completely
> > dynamic, using e.g. the idr subsystem. Static tables are always feel
> > like straight jackets to me, no matter how big they are :)
>
> You are right, I didn't change the code to use idr (it predates idr by
> about a decade or so), because I thought we needed the "rage" logic that
> the usb-serial minor reservation does.
>
> But I'm not so sure anymore, so here's a patch to change to use the idr
> code, and should remove all minor number limitations (well 65k is the
> limit the tty core should be setting I think.)
>
> Tobias, can you test this patch out? Note, I only compiled it, did not
> get the chance to actually run it, so it might not work at all.
I'm afraid this won't work in it's current form. Several drivers and
parts of usb-serial core still depend on the minor numbers being
consecutive for multi-port devices. There are also still references to
SERIAL_TTY_NO_MINOR (255) as well as SERIAL_TTY_MINORS that need to be
addressed.
>
> thanks,
>
> greg k-h
>
> Subject: [PATCH] usb: serial: remove minor number limitation of 255
>
>
> Signed-off-by: Greg Kroah-Hartman
>
> drivers/usb/serial/usb-serial.c | 86
> +---
> 1 file changed, 38 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> index 4753c00..74b6f08 100644
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
> #include "pl2303.h"
>
> #define DRIVER_AUTHOR "Greg Kroah-Hartman "
> @@ -49,7 +50,7 @@
> drivers depend on it.
> */
>
> -static struct usb_serial *serial_table[SERIAL_TTY_MINORS];
> +static DEFINE_IDR(serial_minors);
> static DEFINE_MUTEX(table_lock);
> static LIST_HEAD(usb_serial_driver_list);
>
> @@ -61,59 +62,52 @@ static LIST_HEAD(usb_serial_driver_list);
> struct usb_serial *usb_serial_get_by_index(unsigned index)
> {
> struct usb_serial *serial;
> + struct usb_serial_port *port;
>
> mutex_lock(&table_lock);
> - serial = serial_table[index];
> -
> - if (serial) {
> - mutex_lock(&serial->disc_mutex);
> - if (serial->disconnected) {
> - mutex_unlock(&serial->disc_mutex);
> - serial = NULL;
> - } else {
> - kref_get(&serial->kref);
> - }
> - }
> + port = idr_find(&serial_minors, index);
> mutex_unlock(&table_lock);
> + if (!port)
> + return NULL;
> +
> + serial = port->serial;
> + kref_get(&serial->kref);
> return serial;
> }
We would still need to handle disconnect, and make sure to return with
the disc_mutex held unless disconnected.
>
> -static struct usb_serial *get_free_serial(struct usb_serial *serial,
> - int num_ports, unsigned int *minor)
> +static int get_free_port(struct usb_serial_port *port)
> {
> - unsigned int i, j;
> - int good_spot;
> -
> - dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> + int i;
>
> - *minor = 0;
> mutex_lock(&table_lock);
> - for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
> - if (serial_table[i])
> - continue;
> + i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
> + if (i < 0)
Missing mutex unlock (as already Dave noted).
> + return -EEXIST;
> + port->number = i;
> + mutex_unlock(&table_lock);
> + return i;
> +}
>
> - good_spot = 1;
> - for (j = 1; j <= num_ports-1; ++j)
> - if ((i+j >= SERIAL_TTY_MINORS) || (serial_table[i+j])) {
> - good_spot = 0;
> - i += j;
> - break;
> - }
> - if (good_spot == 0)
> - continue;
> +static int get_free_serial(struct usb_serial *serial, int num_ports,
> +unsigned int *minor)
> +{
> + unsigned int i;
> + unsigned int x;
>
> - *minor = i;
> - j = 0;
> - dev_dbg(&serial->interface->dev, "%s - minor base = %d\n",
> __func__, *minor);
> - for (i = *minor; (i < (*minor + num_ports)) && (i <
> SERIAL_TTY_MINORS); ++i) {
> - serial_table[i] = serial;
> - serial->port[j++]->number = i;
> - }
> - mutex_unlock(&table_lock);
> - return serial;
> + dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> +
> + *minor = 0x;
> + for (i = 0; i < num_ports; ++i) {
> + x = get_free_port(serial->port[i]);
> + if (x < 0)
> +
Re: [PATCH 1/3] USB: serial: ports: add minor and port number
On Wed, Jun 05, 2013 at 10:55:39AM -0700, Greg KH wrote:
> From: Greg Kroah-Hartman
>
> The usb_serial_port structure had the number field, which was the minor
> number for the port, which almost no one really cared about. They
> really wanted the number of the port within the device, which you had to
> subtract from the minor of the parent usb_serial_device structure. To
> clean this up, provide the real minor number of the port, and the number
> of the port within the serial device separately, as these numbers might
> not be related in the future.
>
> Bonus is that this cleans up a lot of logic in the drivers, and saves
> lines overall.
>
> Signed-off-by: Greg Kroah-Hartman
> --- a/drivers/usb/serial/io_edgeport.c
> +++ b/drivers/usb/serial/io_edgeport.c
> @@ -2302,7 +2293,7 @@ static int send_cmd_write_baud_rate(stru
>
> /* Restore original value to disable access to divisor latch */
> MAKE_CMD_WRITE_REG(&currCmd, &cmdLen, number, LCR,
> - edge_port->shadowLCR);
> +edge_port->shadowLCR);
Unintended indentation change?
> status = write_cmd_usb(edge_port, cmdBuffer, cmdLen);
> if (status) {
> --- a/drivers/usb/serial/whiteheat.c
> +++ b/drivers/usb/serial/whiteheat.c
> @@ -649,7 +649,7 @@ static void firm_setup_port(struct tty_s
> struct whiteheat_port_settings port_settings;
> unsigned int cflag = tty->termios.c_cflag;
>
> - port_settings.port = port->number + 1;
> + port_settings.port = port->port_number + 1;
This is a bug that should be fixed separately and backported, as it
prevents port configuration (e.g. set_termios) for ports with minor
number greater than 0.
I took the liberty to prepare a separate patch for v3.10, which you
could rebase the series on.
> /* get the byte size */
> switch (cflag & CSIZE) {
> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -37,7 +37,8 @@
> * @serial: pointer back to the struct usb_serial owner of this port.
> * @port: pointer to the corresponding tty_port for this port.
> * @lock: spinlock to grab when updating portions of this structure.
> - * @number: the number of the port (the minor number).
> + * @minor: the minor number of the port
> + * @port_number: the port number of this struct usb_serial_device (starts at
> 0)
Maybe
@port_number: the struct usb_serial port number of this port (starts at
0)
or something similar, would be more clear?
> * @interrupt_in_buffer: pointer to the interrupt in buffer for this port.
> * @interrupt_in_urb: pointer to the interrupt in struct urb for this port.
> * @interrupt_in_endpointAddress: endpoint address for the interrupt in pipe
Looks good otherwise.
Thanks,
Johan
--
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/
[PATCH] USB: whiteheat: fix broken port configuration
When configuring the port (e.g. set_termios) the port minor number
rather than the port number was used in the request (and they only
coincide for minor number 0).
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/usb/serial/whiteheat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/whiteheat.c b/drivers/usb/serial/whiteheat.c
index b9fca35..347caad 100644
--- a/drivers/usb/serial/whiteheat.c
+++ b/drivers/usb/serial/whiteheat.c
@@ -649,7 +649,7 @@ static void firm_setup_port(struct tty_struct *tty)
struct whiteheat_port_settings port_settings;
unsigned int cflag = tty->termios.c_cflag;
- port_settings.port = port->number + 1;
+ port_settings.port = port->number - port->serial->minor + 1;
/* get the byte size */
switch (cflag & CSIZE) {
--
1.8.2.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/
Re: [PATCH 2/3] USB: serial: make minor allocation dynamic
On Wed, Jun 05, 2013 at 10:54:55AM -0700, Greg KH wrote:
> From: Greg Kroah-Hartman
>
> This moves the allocation of minor device numbers from a static array to
> be dynamic, using the idr interface. This means that you could
> potentially get "gaps" in a minor number range for a single USB serial
> device with multiple ports, but all should still work properly.
>
> Note, we still have the limitation of 255 USB to serial devices in the
> system, as that is all we are registering with the TTY layer at this
> point in time.
>
> Signed-off-by: Greg Kroah-Hartman
[...]
> -static struct usb_serial *get_free_serial(struct usb_serial *serial,
> - int num_ports, unsigned int *minor)
> +static int get_free_port(struct usb_serial_port *port)
> {
> - unsigned int i, j;
> - int good_spot;
> -
> - dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> + int i;
>
> - *minor = 0;
> mutex_lock(&table_lock);
> - for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
> - if (serial_table[i])
> - continue;
> + i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
> + if (i < 0)
> + goto exit;
> + port->minor = i;
> +exit:
> + mutex_unlock(&table_lock);
> + return i;
> +}
>
> - good_spot = 1;
> - for (j = 1; j <= num_ports-1; ++j)
> - if ((i+j >= SERIAL_TTY_MINORS) || (serial_table[i+j])) {
> - good_spot = 0;
> - i += j;
> - break;
> - }
> - if (good_spot == 0)
> - continue;
> +static int get_free_serial(struct usb_serial *serial, int num_ports,
> +unsigned int *minor)
> +{
> + unsigned int i;
> + unsigned int j;
> + int x;
>
> - *minor = i;
> - j = 0;
> - dev_dbg(&serial->interface->dev, "%s - minor base = %d\n",
> __func__, *minor);
> - for (i = *minor; (i < (*minor + num_ports)) && (i <
> SERIAL_TTY_MINORS); ++i, ++j) {
> - serial_table[i] = serial;
> - serial->port[j]->minor = i;
> - serial->port[j]->port_number = i - *minor;
> - }
> - mutex_unlock(&table_lock);
> - return serial;
> + dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> +
> + *minor = 0x;
You could use SERIAL_TTY_NO_MINOR here -- if needed at all, as it has
already been set in create_serial.
> + for (i = 0; i < num_ports; ++i) {
> + x = get_free_port(serial->port[i]);
> + if (x < 0)
> + goto error;
> + if (*minor == 0x)
> + *minor = x;
We must not update *minor until all port minors have been allocated, or
idr_remove might get called for unallocated minors or even minor numbers
of other ports in return_serial when the serial struct is destroyed.
> + serial->port[i]->port_number = i;
> }
> - mutex_unlock(&table_lock);
> - return NULL;
> + return 0;
> +error:
> + /* unwind the already allocated minors */
> + for (j = 0; j < i; ++j)
> + idr_remove(&serial_minors, serial->port[j]->minor);
> + return x;
table_lock?
> }
>
> static void return_serial(struct usb_serial *serial)
> @@ -123,7 +128,7 @@ static void return_serial(struct usb_ser
>
> mutex_lock(&table_lock);
> for (i = 0; i < serial->num_ports; ++i)
> - serial_table[serial->minor + i] = NULL;
> + idr_remove(&serial_minors, serial->port[i]->minor);
> mutex_unlock(&table_lock);
> }
>
[...]
Looks good otherwise.
Thanks,
Johan
--
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/
Re: [PATCH v2 2/3] USB: serial: make minor allocation dynamic
On Thu, Jun 06, 2013 at 10:31:21AM -0700, Greg KH wrote:
> From: Greg Kroah-Hartman
[...]
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
> #include "pl2303.h"
>
> #define DRIVER_AUTHOR "Greg Kroah-Hartman "
> @@ -49,72 +50,75 @@
> drivers depend on it.
> */
>
> -static struct usb_serial *serial_table[SERIAL_TTY_MINORS];
> +static DEFINE_IDR(serial_minors);
> static DEFINE_MUTEX(table_lock);
> static LIST_HEAD(usb_serial_driver_list);
>
> /*
> - * Look up the serial structure. If it is found and it hasn't been
> + * Look up the serial port structure. If it is found and it hasn't been
> * disconnected, return with its disc_mutex held and its refcount
Second sentence needs to be updated as well as it is the disc_mutex and
refcount of the owning usb_serial struct we're referring to.
> * incremented. Otherwise return NULL.
> */
> -struct usb_serial *usb_serial_get_by_index(unsigned index)
> +struct usb_serial_port *usb_serial_port_get_by_minor(unsigned minor)
> {
> - struct usb_serial *serial;
> + struct usb_serial *serial = NULL;
> + struct usb_serial_port *port;
>
> mutex_lock(&table_lock);
> - serial = serial_table[index];
> + port = idr_find(&serial_minors, minor);
> + if (!port)
> + goto exit;
>
> - if (serial) {
> - mutex_lock(&serial->disc_mutex);
> - if (serial->disconnected) {
> - mutex_unlock(&serial->disc_mutex);
> - serial = NULL;
> - } else {
> - kref_get(&serial->kref);
> - }
> + serial = port->serial;
> + mutex_lock(&serial->disc_mutex);
> + if (serial->disconnected) {
> + mutex_unlock(&serial->disc_mutex);
> + serial = NULL;
You want to set port rather than serial to NULL here now.
> + } else {
> + kref_get(&serial->kref);
> }
> +exit:
> mutex_unlock(&table_lock);
> - return serial;
> + return port;
> }
>
> -static struct usb_serial *get_free_serial(struct usb_serial *serial,
> - int num_ports, unsigned int *minor)
> +static int get_free_port(struct usb_serial_port *port)
> {
> - unsigned int i, j;
> - int good_spot;
> -
> - dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> + int i;
>
> - *minor = 0;
> mutex_lock(&table_lock);
> - for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
> - if (serial_table[i])
> - continue;
> + i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
> + if (i < 0)
> + goto exit;
> + port->minor = i;
> +exit:
> + mutex_unlock(&table_lock);
> + return i;
> +}
>
> - good_spot = 1;
> - for (j = 1; j <= num_ports-1; ++j)
> - if ((i+j >= SERIAL_TTY_MINORS) || (serial_table[i+j])) {
> - good_spot = 0;
> - i += j;
> - break;
> - }
> - if (good_spot == 0)
> - continue;
> +static int get_free_serial(struct usb_serial *serial, int num_ports)
> +{
> + unsigned int i;
> + unsigned int j;
> + int x;
>
> - *minor = i;
> - j = 0;
> - dev_dbg(&serial->interface->dev, "%s - minor base = %d\n",
> __func__, *minor);
> - for (i = *minor; (i < (*minor + num_ports)) && (i <
> SERIAL_TTY_MINORS); ++i, ++j) {
> - serial_table[i] = serial;
> - serial->port[j]->minor = i;
> - serial->port[j]->port_number = i - *minor;
> - }
> - mutex_unlock(&table_lock);
> - return serial;
> + dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> +
> + for (i = 0; i < num_ports; ++i) {
> + x = get_free_port(serial->port[i]);
> + if (x < 0)
> + goto error;
> + serial->port[i]->port_number = i;
> }
What do you think about removing get_free_port altogether and simply
call idr_alloc directly in the loop above instead? This would have the
benefit of only acquiring the table_lock once per device, which would
also prevent the possibility of higher port numbers receiving smaller
minors (e.g. due to a parallel disconnect).
> + serial->minors_reserved = 1;
> + return 0;
> +error:
> + /* unwind the already allocated minors */
> + mutex_lock(&table_lock);
> + for (j = 0; j < i; ++j)
> + idr_remove(&serial_minors, serial->port[j]->minor);
> mutex_unlock(&table_lock);
> - return NULL;
> + return x;
> }
[...]
> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -21,7 +21,6 @@
>
> #define SERIAL_TTY_MAJOR 188 /* Nice legal
Re: [PATCH v2 2/3] USB: serial: make minor allocation dynamic
On Fri, Jun 07, 2013 at 11:40:25AM +0200, Johan Hovold wrote: > On Thu, Jun 06, 2013 at 10:31:21AM -0700, Greg KH wrote: > > From: Greg Kroah-Hartman > > [...] > > > /* Functions needed by other parts of the usbserial core */ > > -extern struct usb_serial *usb_serial_get_by_index(unsigned int minor); > > +extern struct usb_serial_port *usb_serial_port_get_by_minor(unsigned int > > minor); > > As Tobias already noted, this breaks the USB console driver. However, it > looks like the call in that driver is not really needed and that > therefore usb_serial_port_get_by_minor need not be exported at all. Forget that last sentence. It is still needed, of course. Johan -- 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/
Re: [PATCH v2 2/3] USB: serial: make minor allocation dynamic
On Thu, Jun 06, 2013 at 10:31:21AM -0700, Greg KH wrote:
> From: Greg Kroah-Hartman
> @@ -1040,11 +1044,10 @@ static int usb_serial_probe(struct usb_i
>*/
> serial->disconnected = 1;
>
> - if (get_free_serial(serial, num_ports, &minor) == NULL) {
> + if (get_free_serial(serial, num_ports)) {
> dev_err(ddev, "No more free serial devices\n");
> goto probe_error;
> }
> - serial->minor = minor;
This gives a warning as minor is no longer initialised, but is still
used to initialise the console a bit further down.
usb_serial_console_init(minor);
Should probably just drop minor, and use serial->port[0]->minor instead.
Thanks,
Johan
--
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/
Re: [PATCH v3 2/3] USB: serial: make minor allocation dynamic
> > > @@ -123,8 +116,9 @@ static void return_serial(struct usb_ser > > > > > > mutex_lock(&table_lock); > > > for (i = 0; i < serial->num_ports; ++i) > > > - serial_table[serial->minor + i] = NULL; > > > + idr_remove(&serial_minors, serial->port[i]->minor); > > > mutex_unlock(&table_lock); > > > + serial->minors_reserved = 0; > > > > This isn't strictly needed as the serial struct release_serial is only > > called once when the struct is about to be freed. > > Really? Why were we doing this type of thing before with the "not > allocated" flag? It seems that we were protecting some path that I > can't remember at the moment. So to be safe, I'll leave it for now... It was and is only used when releasing the serial struct to check whether minors had been allocated or not at probe and if return_serial (release_minors) should be called. This in done in destroy_serial just before freeing the struct, so clearing the flag is redundant, but doesn't hurt anyone, I guess. ;) Johan -- 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/
Re: [PATCH v3 2/3] USB: serial: make minor allocation dynamic
*/ > + for (j = 0; j < i; ++j) > + idr_remove(&serial_minors, serial->port[j]->minor); > + mutex_unlock(&table_lock); > + return minor; > } > > static void return_serial(struct usb_serial *serial) Perhaps rename this one release_minors to match allocate_minors (much better name btw)? > @@ -123,8 +116,9 @@ static void return_serial(struct usb_ser > > mutex_lock(&table_lock); > for (i = 0; i < serial->num_ports; ++i) > - serial_table[serial->minor + i] = NULL; > + idr_remove(&serial_minors, serial->port[i]->minor); > mutex_unlock(&table_lock); > + serial->minors_reserved = 0; This isn't strictly needed as the serial struct release_serial is only called once when the struct is about to be freed. > } > > static void destroy_serial(struct kref *kref) > @@ -136,7 +130,7 @@ static void destroy_serial(struct kref * > serial = to_usb_serial(kref); > > /* return the minor range that this device had */ > - if (serial->minor != SERIAL_TTY_NO_MINOR) > + if (serial->minors_reserved) > return_serial(serial); > > if (serial->attached && serial->type->release) All three patches look good otherwise. The port-number disambiguation was indeed long overdue. Feel free to add Reviewed-by: Johan Hovold Thanks, Johan -- 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/
Re: [PATCH 0/2] *** SUBJECT HERE ***
On Tue, Jul 02, 2013 at 01:22:01AM +0200, Anders Hammarquist wrote: > In a message of Fri, 28 Jun 2013 12:23:33 +0200, Johan Hovold writes: > >> I did a quick check of adding the device id though sysfs, and although > >> it partly works, it doesn't find the correct firmware (it ends up trying > >> to load 5052 firmware for a 3410 device. Looking at the code it seems > >> (struct ti_device) td_is_3410 isn't set properly.) > > > >Turns out that the drivers device-type detection has never worked with > >the dynamic id interface (all devices were detected as 2-port devices). > > > >I'm responding to this mail with a fix. Care to give it a try? > > Yes, this works fine. Thanks for testing. Johan -- 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/
Re: [PATCH 0/2] *** SUBJECT HERE ***
On Wed, Jun 26, 2013 at 10:29:59AM +0200, Anders Hammarquist wrote:
> In a message of Tue, 25 Jun 2013 16:39:11 -0700, Greg KH writes:
> >> Indeed. I'd already had some (failed) thoughts about how to handle it
> >> nicely. Now I've had another think through, and I have something which
> >> deals with it and at least complains if TI_EXTRA_VID_PID_COUNT is changed
> >> without changing the initializer. Patch 2/2
> >
> >Why don't we just drop the extra id thing entirely? The usb-serial
> >subsystem handles new device ids being added dynamically from sysfs for
> >a long time now. Removing this module option would clean up the code a
> >lot, and prevent these errors from ever happening again.
>
> Aha, yes, I'm all for that (had I only known I'd have done that to start
> with). I'll look in to it.
I already have a few patches here (part of a larger 3.11 clean-up series)
which removes the vid/pid module parameters from all usb-serial modules
including ti_usb_3410_5052.
I hope to be able to submit the whole series a later tonight, but here's
the ti_usb_3410_5052 part if anyone's interested.
Thanks,
Johan
From: Johan Hovold
Subject: [PATCH] USB: ti_usb_3410_5052: remove vendor/product module parameters
Remove the vendor and product module parameters which were added a long
time ago when we did not have the dynamic sysfs interface to add
new device ids (and which isn't limited to five new vid/pid pair).
A vid/pid pair can be added dynamically using sysfs, for example:
echo 0451 1234 >/sys/bus/usb-serial/drivers/ti_usb_3410_5052_1/new_id
for 1-port adapters, or
echo 0451 1234 >/sys/bus/usb-serial/drivers/ti_usb_3410_5052_2/new_id
for 2-port adapters.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ti_usb_3410_5052.c | 72 ---
1 file changed, 7 insertions(+), 65 deletions(-)
diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
b/drivers/usb/serial/ti_usb_3410_5052.c
index f3e21f5..5585b20 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -141,20 +141,9 @@ static int ti_download_firmware(struct ti_device *tdev);
/* module parameters */
static int closing_wait = TI_DEFAULT_CLOSING_WAIT;
-static ushort vendor_3410[TI_EXTRA_VID_PID_COUNT];
-static unsigned int vendor_3410_count;
-static ushort product_3410[TI_EXTRA_VID_PID_COUNT];
-static unsigned int product_3410_count;
-static ushort vendor_5052[TI_EXTRA_VID_PID_COUNT];
-static unsigned int vendor_5052_count;
-static ushort product_5052[TI_EXTRA_VID_PID_COUNT];
-static unsigned int product_5052_count;
/* supported devices */
-/* the array dimension is the number of default entries plus */
-/* TI_EXTRA_VID_PID_COUNT user defined entries plus 1 terminating */
-/* null entry */
-static struct usb_device_id ti_id_table_3410[15+TI_EXTRA_VID_PID_COUNT+1] = {
+static struct usb_device_id ti_id_table_3410[] = {
{ USB_DEVICE(TI_VENDOR_ID, TI_3410_PRODUCT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, TI_3410_EZ430_ID) },
{ USB_DEVICE(MTS_VENDOR_ID, MTS_GSM_NO_FW_PRODUCT_ID) },
@@ -171,16 +160,18 @@ static struct usb_device_id
ti_id_table_3410[15+TI_EXTRA_VID_PID_COUNT+1] = {
{ USB_DEVICE(ABBOTT_VENDOR_ID, ABBOTT_STEREO_PLUG_ID) },
{ USB_DEVICE(ABBOTT_VENDOR_ID, ABBOTT_STRIP_PORT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, FRI2_PRODUCT_ID) },
+ { } /* terminator */
};
-static struct usb_device_id ti_id_table_5052[5+TI_EXTRA_VID_PID_COUNT+1] = {
+static struct usb_device_id ti_id_table_5052[] = {
{ USB_DEVICE(TI_VENDOR_ID, TI_5052_BOOT_PRODUCT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, TI_5152_BOOT_PRODUCT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, TI_5052_EEPROM_PRODUCT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, TI_5052_FIRMWARE_PRODUCT_ID) },
+ { } /* terminator */
};
-static struct usb_device_id
ti_id_table_combined[19+2*TI_EXTRA_VID_PID_COUNT+1] = {
+static struct usb_device_id ti_id_table_combined[] = {
{ USB_DEVICE(TI_VENDOR_ID, TI_3410_PRODUCT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, TI_3410_EZ430_ID) },
{ USB_DEVICE(MTS_VENDOR_ID, MTS_GSM_NO_FW_PRODUCT_ID) },
@@ -200,7 +191,7 @@ static struct usb_device_id
ti_id_table_combined[19+2*TI_EXTRA_VID_PID_COUNT+1]
{ USB_DEVICE(IBM_VENDOR_ID, IBM_454C_PRODUCT_ID) },
{ USB_DEVICE(ABBOTT_VENDOR_ID, ABBOTT_PRODUCT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, FRI2_PRODUCT_ID) },
- { }
+ { } /* terminator */
};
static struct usb_serial_driver ti_1port_device = {
@@ -289,61 +280,12 @@ module_param(closing_wait, int, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(closing_wait,
"Maximum wait for data to drain in close, in .01 secs, default is 4000");
-module_param_array(vendor_3410, ushort, &vendor_3410_count, S_IRUGO);
-MODULE_PARM_DESC(vendor_3410,
- "
Re: [PATCH 0/2] *** SUBJECT HERE ***
On Thu, Jun 27, 2013 at 11:50:52PM +0200, Anders Hammarquist wrote: > In a message of Wed, 26 Jun 2013 12:39:24 +0200, Johan Hovold writes: > >On Wed, Jun 26, 2013 at 10:29:59AM +0200, Anders Hammarquist wrote: > >> In a message of Tue, 25 Jun 2013 16:39:11 -0700, Greg KH writes: > >> >> Indeed. I'd already had some (failed) thoughts about how to handle it > >> >> nicely. Now I've had another think through, and I have something which > >> >> deals with it and at least complains if TI_EXTRA_VID_PID_COUNT is > >> >> changed > >> >> without changing the initializer. Patch 2/2 > >> > > >> >Why don't we just drop the extra id thing entirely? The usb-serial > >> >subsystem handles new device ids being added dynamically from sysfs for > >> >a long time now. Removing this module option would clean up the code a > >> >lot, and prevent these errors from ever happening again. > >> > >> Aha, yes, I'm all for that (had I only known I'd have done that to start > >> with). I'll look in to it. > > > >I already have a few patches here (part of a larger 3.11 clean-up series) > >which removes the vid/pid module parameters from all usb-serial modules > >including ti_usb_3410_5052. > > > >I hope to be able to submit the whole series a later tonight, but here's > >the ti_usb_3410_5052 part if anyone's interested. > > I did a quick check of adding the device id though sysfs, and although > it partly works, it doesn't find the correct firmware (it ends up trying > to load 5052 firmware for a 3410 device. Looking at the code it seems > (struct ti_device) td_is_3410 isn't set properly.) Turns out that the drivers device-type detection has never worked with the dynamic id interface (all devices were detected as 2-port devices). I'm responding to this mail with a fix. Care to give it a try? Thanks, Johan -- 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/
[PATCH] USB: ti_usb_3410_5052: fix dynamic-id matching
The driver failed to take the dynamic ids into account when determining the device type and therefore all devices were detected as 2-port devices when using the dynamic-id interface. Match on the usb-serial-driver field instead of doing redundant id-table searches. Reported-by: Anders Hammarquist Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ti_usb_3410_5052.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c index 5585b20..5c07d55 100644 --- a/drivers/usb/serial/ti_usb_3410_5052.c +++ b/drivers/usb/serial/ti_usb_3410_5052.c @@ -309,7 +309,7 @@ static int ti_startup(struct usb_serial *serial) usb_set_serial_data(serial, tdev); /* determine device type */ - if (usb_match_id(serial->interface, ti_id_table_3410)) + if (serial->type == &ti_1port_device) tdev->td_is_3410 = 1; dev_dbg(&dev->dev, "%s - device type is %s\n", __func__, tdev->td_is_3410 ? "3410" : "5052"); -- 1.8.2.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/
Re: [PATCH 098/150] USB: serial: add modem-status-change wait queue
On Tue, Mar 26, 2013 at 03:19:57PM +, Luis Henriques wrote:
> 3.5.7.9 -stable review patch. If anyone has any objections, please let me
> know.
This patch is incorrect as the wait-queue initialisation is missing. A
fix has been posted to linux-usb:
http://marc.info/?l=linux-usb&m=136428758202815&w=2
and should show up in 3.9-rc5. This patch and the following
use-after-free patches should not be applied without that fix.
Johan
> --
>
> From: Johan Hovold
>
> commit e5b33dc9d16053c2ae4c2c669cf008829530364b upstream.
>
> Add modem-status-change wait queue to struct usb_serial_port that
> subdrivers can use to implement TIOCMIWAIT.
>
> Currently subdrivers use a private wait queue which may have been
> released when waking up after device disconnected.
>
> Note that we're adding a new wait queue rather than reusing the tty-port
> one as we do not want to get woken up at hangup (yet).
>
> Signed-off-by: Johan Hovold
> Signed-off-by: Greg Kroah-Hartman
> Signed-off-by: Luis Henriques
> ---
> include/linux/usb/serial.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
> index 86c0b45..0b61f01 100644
> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -66,6 +66,7 @@
> * port.
> * @flags: usb serial port flags
> * @write_wait: a wait_queue_head_t used by the port.
> + * @delta_msr_wait: modem-status-change wait queue
> * @work: work queue entry for the line discipline waking up.
> * @throttled: nonzero if the read urb is inactive to throttle the device
> * @throttle_req: nonzero if the tty wants to throttle us
> @@ -112,6 +113,7 @@ struct usb_serial_port {
>
> unsigned long flags;
> wait_queue_head_t write_wait;
> + wait_queue_head_t delta_msr_wait;
> struct work_struct work;
> charthrottled;
> charthrottle_req;
--
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/
Re: [ 084/104] USB: serial: add modem-status-change wait queue
On Mon, Mar 25, 2013 at 2:06 AM, Ben Hutchings wrote:
> 3.2-stable review patch. If anyone has any objections, please let me know.
This patch is incorrect as the wait-queue initialisation is missing. A
fix has been posted to linux-usb:
http://marc.info/?l=linux-usb&m=136428758202815&w=2
and should show up in 3.9-rc5. This patch and the following
use-after-free patches should not be applied without that fix.
Johan
> ------
>
> From: Johan Hovold
>
> commit e5b33dc9d16053c2ae4c2c669cf008829530364b upstream.
>
> Add modem-status-change wait queue to struct usb_serial_port that
> subdrivers can use to implement TIOCMIWAIT.
>
> Currently subdrivers use a private wait queue which may have been
> released when waking up after device disconnected.
>
> Note that we're adding a new wait queue rather than reusing the tty-port
> one as we do not want to get woken up at hangup (yet).
>
> Signed-off-by: Johan Hovold
> Signed-off-by: Greg Kroah-Hartman
> Signed-off-by: Ben Hutchings
> ---
> include/linux/usb/serial.h |2 ++
> 1 file changed, 2 insertions(+)
>
> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -71,6 +71,7 @@ enum port_dev_state {
> * port.
> * @flags: usb serial port flags
> * @write_wait: a wait_queue_head_t used by the port.
> + * @delta_msr_wait: modem-status-change wait queue
> * @work: work queue entry for the line discipline waking up.
> * @throttled: nonzero if the read urb is inactive to throttle the device
> * @throttle_req: nonzero if the tty wants to throttle us
> @@ -114,6 +115,7 @@ struct usb_serial_port {
>
> unsigned long flags;
> wait_queue_head_t write_wait;
> + wait_queue_head_t delta_msr_wait;
> struct work_struct work;
> charthrottled;
> charthrottle_req;
>
>
--
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/
Re: [PATCH] rtc: rtc-at91rm9200: use a variable for storing IMR
On Fri, Mar 15, 2013 at 06:37:12PM +0100, Nicolas Ferre wrote:
> On some revisions of AT91 SoCs, the RTC IMR register is not working.
> Instead of elaborating a workaround for that specific SoC or IP version,
> we simply use a software variable to store the Interrupt Mask Register and
> modify it for each enabling/disabling of an interrupt. The overhead of this
> is negligible anyway.
The patch does not add any memory barriers or register read-backs when
manipulating the interrupt-mask variable. This could possibly lead to
spurious interrupts both when enabling and disabling the various
RTC-interrupts due to write reordering and bus latencies.
Has this been considered? And is this reason enough for a more targeted
work-around so that the SOCs with functional RTC_IMR are not affected?
> Reported-by: Douglas Gilbert
> Signed-off-by: Nicolas Ferre
> ---
> drivers/rtc/rtc-at91rm9200.c | 50
> +++-
> drivers/rtc/rtc-at91rm9200.h | 1 -
> 2 files changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
> index 79233d0..29b92e4 100644
> --- a/drivers/rtc/rtc-at91rm9200.c
> +++ b/drivers/rtc/rtc-at91rm9200.c
> @@ -46,6 +46,7 @@ static DECLARE_COMPLETION(at91_rtc_updated);
> static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
> static void __iomem *at91_rtc_regs;
> static int irq;
> +static u32 at91_rtc_imr;
>
> /*
>
> * Decode time/date into rtc_time structure
[...]
> @@ -198,9 +203,12 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
> unsigned int enabled)
>
> if (enabled) {
> at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
> + at91_rtc_imr |= AT91_RTC_ALARM;
wmb() needed before enabling interrupt as at91_rtc_write() uses
__raw_writel() which does not add any barriers?
> at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
> - } else
> + } else {
> at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
wmb() and register read-back needed before updating interrupt mask?
> + at91_rtc_imr &= ~AT91_RTC_ALARM;
> + }
>
> return 0;
> }
[...]
> @@ -229,7 +235,7 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
> *dev_id)
> unsigned int rtsr;
> unsigned long events = 0;
>
> - rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read(AT91_RTC_IMR);
> + rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_imr;
Does at91_rtc_imr necessarily match the hardware state here?
> if (rtsr) { /* this interrupt is shared! Is it ours? */
> if (rtsr & AT91_RTC_ALARM)
> events |= (RTC_AF | RTC_IRQF);
Johan
--
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/
Re: [PATCH 0/5] at91: atmel_lcdfb: regression fixes and cpu_is removal
On Sat, Feb 09, 2013 at 04:47:40PM -0800, Olof Johansson wrote: > On Fri, Feb 08, 2013 at 05:35:13PM +0100, Nicolas Ferre wrote: > > These patches fix a regression in 16-bpp support for older SOCs which use > > IBGR:555 rather than BGR:565 pixel layout. Use SOC-type to determine if the > > controller uses the intensity-bit and restore the old layout in that case. > > > > The last patch is a removal of uses of cpu_is_() macros in atmel_lcdfb > > with > > a platform-device-id table and static configurations. > > > > > > Patches from Johan Hovold taken from: > > "[PATCH 0/3] atmel_lcdfb: fix 16-bpp regression" > > and > > "[PATCH v2 0/3] ARM: at91/avr32/atmel_lcdfb: remove cpu_is macros" > > patch series to form a clean patch series with my signature. > > > > Arnd, Olof, > > as it seems that old fbdev drivers are not so much reviewed those days, can > > we > > take the decision to queue this material through arm-soc with other AT91 > > drivers updates? > > It would be beneficial to get an ack from Florian. Was he involved in the > review of the code that regressed 16-bpp support in the first place? When was > the regression introduced? Thought I'd send a reminder about these fixes. Has anyone picked them up for 3.9-rc? Thanks, Johan -- 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/
[PATCH 2/2] ARM: at91: remove trailing semicolon from macros
Remove trailing semicolon from register-access macros. Signed-off-by: Johan Hovold --- arch/arm/mach-at91/at91_rstc.h| 2 +- arch/arm/mach-at91/at91_shdwc.h | 2 +- arch/arm/mach-at91/at91x40_time.c | 2 +- arch/arm/mach-at91/include/mach/at91_matrix.h | 2 +- arch/arm/mach-at91/include/mach/at91_st.h | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/arm/mach-at91/at91_rstc.h b/arch/arm/mach-at91/at91_rstc.h index 875fa33..a600e69 100644 --- a/arch/arm/mach-at91/at91_rstc.h +++ b/arch/arm/mach-at91/at91_rstc.h @@ -23,7 +23,7 @@ extern void __iomem *at91_rstc_base; __raw_readl(at91_rstc_base + field) #define at91_rstc_write(field, value) \ - __raw_writel(value, at91_rstc_base + field); + __raw_writel(value, at91_rstc_base + field) #else .extern at91_rstc_base #endif diff --git a/arch/arm/mach-at91/at91_shdwc.h b/arch/arm/mach-at91/at91_shdwc.h index 60478ea..9e29f31 100644 --- a/arch/arm/mach-at91/at91_shdwc.h +++ b/arch/arm/mach-at91/at91_shdwc.h @@ -23,7 +23,7 @@ extern void __iomem *at91_shdwc_base; __raw_readl(at91_shdwc_base + field) #define at91_shdwc_write(field, value) \ - __raw_writel(value, at91_shdwc_base + field); + __raw_writel(value, at91_shdwc_base + field) #endif #define AT91_SHDW_CR 0x00/* Shut Down Control Register */ diff --git a/arch/arm/mach-at91/at91x40_time.c b/arch/arm/mach-at91/at91x40_time.c index 0e57e44..f4c81c2 100644 --- a/arch/arm/mach-at91/at91x40_time.c +++ b/arch/arm/mach-at91/at91x40_time.c @@ -33,7 +33,7 @@ __raw_readl(AT91_IO_P2V(AT91_TC) + field) #define at91_tc_write(field, value) \ - __raw_writel(value, AT91_IO_P2V(AT91_TC) + field); + __raw_writel(value, AT91_IO_P2V(AT91_TC) + field) /* * 3 counter/timer units present. diff --git a/arch/arm/mach-at91/include/mach/at91_matrix.h b/arch/arm/mach-at91/include/mach/at91_matrix.h index 02fae9d..f8996c9 100644 --- a/arch/arm/mach-at91/include/mach/at91_matrix.h +++ b/arch/arm/mach-at91/include/mach/at91_matrix.h @@ -14,7 +14,7 @@ extern void __iomem *at91_matrix_base; __raw_readl(at91_matrix_base + field) #define at91_matrix_write(field, value) \ - __raw_writel(value, at91_matrix_base + field); + __raw_writel(value, at91_matrix_base + field) #else .extern at91_matrix_base diff --git a/arch/arm/mach-at91/include/mach/at91_st.h b/arch/arm/mach-at91/include/mach/at91_st.h index 969aac2..67fdbd1 100644 --- a/arch/arm/mach-at91/include/mach/at91_st.h +++ b/arch/arm/mach-at91/include/mach/at91_st.h @@ -23,7 +23,7 @@ extern void __iomem *at91_st_base; __raw_readl(at91_st_base + field) #define at91_st_write(field, value) \ - __raw_writel(value, at91_st_base + field); + __raw_writel(value, at91_st_base + field) #else .extern at91_st_base #endif -- 1.8.1.5 -- 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/
[PATCH 1/2] ARM: at91/setup: fix trivial typos
Fix a few trivial typos in panic, warning and debug messages.
Signed-off-by: Johan Hovold
---
arch/arm/mach-at91/setup.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
index 4b67847..9e7c1e1 100644
--- a/arch/arm/mach-at91/setup.c
+++ b/arch/arm/mach-at91/setup.c
@@ -333,7 +333,7 @@ static void at91_dt_rstc(void)
of_id = of_match_node(rstc_ids, np);
if (!of_id)
- panic("AT91: rtsc no restart function availlable\n");
+ panic("AT91: rtsc no restart function available\n");
arm_pm_restart = of_id->data;
@@ -353,7 +353,7 @@ static void at91_dt_ramc(void)
np = of_find_matching_node(NULL, ramc_ids);
if (!np)
- panic("unable to find compatible ram conroller node in dtb\n");
+ panic("unable to find compatible ram controller node in dtb\n");
at91_ramc_base[0] = of_iomap(np, 0);
if (!at91_ramc_base[0])
@@ -403,7 +403,7 @@ static void at91_dt_shdwc(void)
np = of_find_matching_node(NULL, shdwc_ids);
if (!np) {
- pr_debug("AT91: unable to find compatible shutdown (shdwc)
conroller node in dtb\n");
+ pr_debug("AT91: unable to find compatible shutdown (shdwc)
controller node in dtb\n");
return;
}
@@ -419,7 +419,7 @@ static void at91_dt_shdwc(void)
if (!of_property_read_u32(np, "atmel,wakeup-counter", ®)) {
if (reg > AT91_SHDW_CPTWK0_MAX) {
- pr_warn("AT91: shdwc wakeup conter 0x%x > 0x%x reduce
it to 0x%x\n",
+ pr_warn("AT91: shdwc wakeup counter 0x%x > 0x%x reduce
it to 0x%x\n",
reg, AT91_SHDW_CPTWK0_MAX,
AT91_SHDW_CPTWK0_MAX);
reg = AT91_SHDW_CPTWK0_MAX;
}
--
1.8.1.5
--
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/
Re: [PATCH 035/102] USB: serial: add modem-status-change wait queue
On Mon, Apr 08, 2013 at 10:49:50AM +0100, Luis Henriques wrote:
> 3.5.7.10 -stable review patch. If anyone has any objections, please let me
> know.
>
> --
>
> From: Johan Hovold
>
> commit e5b33dc9d16053c2ae4c2c669cf008829530364b upstream.
>
> Add modem-status-change wait queue to struct usb_serial_port that
> subdrivers can use to implement TIOCMIWAIT.
>
> Currently subdrivers use a private wait queue which may have been
> released when waking up after device disconnected.
>
> Note that we're adding a new wait queue rather than reusing the tty-port
> one as we do not want to get woken up at hangup (yet).
This one should be followed by eba0e3c3a0ba7b96f0 (USB: serial: fix hang
when opening port) which adds the missing queue initialisation. It
appears the next patch in your series starts using the wait queue
directly.
Thanks,
Johan
> Signed-off-by: Johan Hovold
> Signed-off-by: Greg Kroah-Hartman
> Signed-off-by: Luis Henriques
> ---
> include/linux/usb/serial.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
> index 86c0b45..0b61f01 100644
> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -66,6 +66,7 @@
> * port.
> * @flags: usb serial port flags
> * @write_wait: a wait_queue_head_t used by the port.
> + * @delta_msr_wait: modem-status-change wait queue
> * @work: work queue entry for the line discipline waking up.
> * @throttled: nonzero if the read urb is inactive to throttle the device
> * @throttle_req: nonzero if the tty wants to throttle us
> @@ -112,6 +113,7 @@ struct usb_serial_port {
>
> unsigned long flags;
> wait_queue_head_t write_wait;
> + wait_queue_head_t delta_msr_wait;
> struct work_struct work;
> charthrottled;
> charthrottle_req;
--
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/
Re: Regression: ftdi_sio is slow (since Wed Oct 10 15:05:06 2012)
On Sat, May 04, 2013 at 01:50:42AM +0400, Stas Sergeev wrote: > 04.05.2013 00:34, Greg KH пишет: > > On Fri, May 03, 2013 at 10:27:18PM +0400, Stas Sergeev wrote: > >> 03.05.2013 21:16, Greg KH пишет: [...] > >>> There's no guarantee as to how long select or an ioctl will take, and > >>> now that we have fixed another bug, this device is slower. > >>> > >>> If you change hardware types to use a different usb to serial chip, that > >>> select call might take 4 times as long. Are we somehow supposed to > >>> change the kernel to "fix" that? > >> Previously, the kernel was not calling to a device at all, so > >> select() was independent of the chip, and it was fast. I was > >> not aware you changed that willingly. > > I don't understand, what do you mean by this? Some drivers just return > > the value of an internally held number, and don't query the device. > > > > The only way the FTDI driver can determine if the hardware buffer on the > > chip way out on the end of the USB cable is empty or not, is to query > > it. So the driver now does so. > It does so only for one char. And the query takes longer than > to just xmit that char. So why do you think this even works as > expected? The query takes longer than the transmit at decent baudrates (>=38k) and under the assumption that flow control isn't causing any delays. But you do have a point, and I have been meaning to look into whether the added overhead of checking the hardware buffers could be mitigated by adding wait_until_sent support to usb-serial. This way the we would only query the hardware buffers on tty_wait_until_sent (e.g. at close) and select and TIOCMOUTQ would not suffer. This is also the way things are handled in serial_core. I'll prepare a series which adds wait_until_sent to usb-serial, but I doubt it would be stable material (even if it could get into 3.10). What do you think Greg, is this overhead to chars_in_buffer reason enough to disable it in the stable trees or should we simply fix it in 3.11 (or 3.10)? (The overhead is about 3-400 us per call when the port fifo is empty, which makes chars_in_buffer about 100 times slower on my test system.) Thanks, Johan -- 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/
Re: Regression: ftdi_sio is slow (since Wed Oct 10 15:05:06 2012)
On Sat, May 04, 2013 at 07:39:57AM -0400, Peter Hurley wrote: > On 05/04/2013 07:15 AM, Johan Hovold wrote: > > On Sat, May 04, 2013 at 01:50:42AM +0400, Stas Sergeev wrote: > >> 04.05.2013 00:34, Greg KH пишет: > >>> On Fri, May 03, 2013 at 10:27:18PM +0400, Stas Sergeev wrote: > >>>> 03.05.2013 21:16, Greg KH пишет: [...] > > But you do have a point, and I have been meaning to look into whether > > the added overhead of checking the hardware buffers could be mitigated > > by adding wait_until_sent support to usb-serial. This way the we would > > only query the hardware buffers on tty_wait_until_sent (e.g. at close) > > and select and TIOCMOUTQ would not suffer. This is also the way things > > are handled in serial_core. > > Agreed. This is the correct solution. > > > I'll prepare a series which adds wait_until_sent to usb-serial, but I > > doubt it would be stable material (even if it could get into 3.10). > > > > What do you think Greg, is this overhead to chars_in_buffer reason > > enough to disable it in the stable trees or should we simply fix it in > > 3.11 (or 3.10)? (The overhead is about 3-400 us per call when the port > > fifo is empty, which makes chars_in_buffer about 100 times slower on my > > test system.) > > A better solution for stable would be to set port->drain_delay. It > won't help tcdrain() but at least the port won't shutdown on live > outbound data. Removing the hardware-buffer checks from chars_in_buffer would mean breaking tty_wait_until_sent and thus also, for example, tcdrain, tcsendbreak, and close. Using drain_delay would partially work-around the problem with close, but at the cost of adding delays for all users while still not being able to guarantee that the buffers have been drained. Either way, this seems to amount to a regression. On the other hand, the added overhead to chars_in_buffer seems to break at least one application as well. I've implemented wait_until_sent-support for usb-serial, which fixes the problem without any such trade-offs and which I believe needs to go in to v3.10. For the stable trees I think keeping the current, less efficient (but more complete) chars_in_buffer implementations is preferred over introducing further regressions. Back-porting wait_until_sent-support could perhaps also be considered. I'm responding to this mail with a wait_until_sent-support series. Thanks, Johan -- 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/
[PATCH 6/7] USB: ti_usb_3410_5052: fix chars_in_buffer overhead
Use the new generic usb-serial wait_until_sent implementation to wait
for hardware buffers to drain.
This removes the need to check the hardware buffers in chars_in_buffer
and thus removes the overhead introduced by commit 2c992cd73 ("USB:
ti_usb_3410_5052: query hardware-buffer status in chars_in_buffer")
without breaking tty_wait_until_sent (used by, for example, tcdrain,
tcsendbreak and close).
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ti_usb_3410_5052.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
b/drivers/usb/serial/ti_usb_3410_5052.c
index cac47ae..c92c5ed 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -101,6 +101,7 @@ static int ti_write(struct tty_struct *tty, struct
usb_serial_port *port,
const unsigned char *data, int count);
static int ti_write_room(struct tty_struct *tty);
static int ti_chars_in_buffer(struct tty_struct *tty);
+static bool ti_tx_empty(struct usb_serial_port *port);
static void ti_throttle(struct tty_struct *tty);
static void ti_unthrottle(struct tty_struct *tty);
static int ti_ioctl(struct tty_struct *tty,
@@ -222,6 +223,7 @@ static struct usb_serial_driver ti_1port_device = {
.write = ti_write,
.write_room = ti_write_room,
.chars_in_buffer= ti_chars_in_buffer,
+ .tx_empty = ti_tx_empty,
.throttle = ti_throttle,
.unthrottle = ti_unthrottle,
.ioctl = ti_ioctl,
@@ -253,6 +255,7 @@ static struct usb_serial_driver ti_2port_device = {
.write = ti_write,
.write_room = ti_write_room,
.chars_in_buffer= ti_chars_in_buffer,
+ .tx_empty = ti_tx_empty,
.throttle = ti_throttle,
.unthrottle = ti_unthrottle,
.ioctl = ti_ioctl,
@@ -684,8 +687,6 @@ static int ti_chars_in_buffer(struct tty_struct *tty)
struct ti_port *tport = usb_get_serial_port_data(port);
int chars = 0;
unsigned long flags;
- int ret;
- u8 lsr;
if (tport == NULL)
return 0;
@@ -694,16 +695,22 @@ static int ti_chars_in_buffer(struct tty_struct *tty)
chars = kfifo_len(&tport->write_fifo);
spin_unlock_irqrestore(&tport->tp_lock, flags);
- if (!chars) {
- ret = ti_get_lsr(tport, &lsr);
- if (!ret && !(lsr & TI_LSR_TX_EMPTY))
- chars = 1;
- }
-
dev_dbg(&port->dev, "%s - returns %d\n", __func__, chars);
return chars;
}
+static bool ti_tx_empty(struct usb_serial_port *port)
+{
+ struct ti_port *tport = usb_get_serial_port_data(port);
+ int ret;
+ u8 lsr;
+
+ ret = ti_get_lsr(tport, &lsr);
+ if (!ret && !(lsr & TI_LSR_TX_EMPTY))
+ return false;
+
+ return true;
+}
static void ti_throttle(struct tty_struct *tty)
{
--
1.8.2.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/
[PATCH 3/7] USB: ftdi_sio: clean up get_modem_status
Use usb-serial port rather than tty as argument to get_modem_status.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 242b577..1159fd4 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -925,7 +925,7 @@ static int ftdi_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg);
static void ftdi_break_ctl(struct tty_struct *tty, int break_state);
static int ftdi_chars_in_buffer(struct tty_struct *tty);
-static int ftdi_get_modem_status(struct tty_struct *tty,
+static int ftdi_get_modem_status(struct usb_serial_port *port,
unsigned char status[2]);
static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base);
@@ -2068,7 +2068,7 @@ static int ftdi_chars_in_buffer(struct tty_struct *tty)
goto out;
/* Check if hardware buffer is empty. */
- ret = ftdi_get_modem_status(tty, buf);
+ ret = ftdi_get_modem_status(port, buf);
if (ret == 2) {
if (!(buf[1] & FTDI_RS_TEMT))
chars = 1;
@@ -2268,10 +2268,9 @@ no_c_cflag_changes:
* Returns the number of status bytes retrieved (device dependant), or
* negative error code.
*/
-static int ftdi_get_modem_status(struct tty_struct *tty,
+static int ftdi_get_modem_status(struct usb_serial_port *port,
unsigned char status[2])
{
- struct usb_serial_port *port = tty->driver_data;
struct ftdi_private *priv = usb_get_serial_port_data(port);
unsigned char *buf;
int len;
@@ -2336,7 +2335,7 @@ static int ftdi_tiocmget(struct tty_struct *tty)
unsigned char buf[2];
int ret;
- ret = ftdi_get_modem_status(tty, buf);
+ ret = ftdi_get_modem_status(port, buf);
if (ret < 0)
return ret;
--
1.8.2.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/
[PATCH 0/7] USB: serial: add wait_until_sent-support
These patches add wait_until_sent-support to usb-serial, which removes the need to check hardware buffers in chars_in_buffer. This fixes a problem in ftdi_sio (since 3.7) where select or TIOCMOUTQ would take much longer than before due the hardware buffers being queried. Hardware buffers are also currently checked in chars_in_buffer in io_ti (since 3.8) and ti_usb_3410_5052 (in 3.10). Note that simply removing the hardware-buffer checks (e.g. for the stable trees) would break tty_wait_until_sent, which is used, for instance, by tcdrain, tcsendbreak, and close. Johan Johan Hovold (7): USB: serial: add wait_until_sent operation USB: serial: add generic wait_until_sent implementation USB: ftdi_sio: clean up get_modem_status USB: ftdi_sio: fix chars_in_buffer overhead USB: io_ti: fix chars_in_buffer overhead USB: ti_usb_3410_5052: fix chars_in_buffer overhead USB: serial: clean up chars_in_buffer drivers/usb/serial/ftdi_sio.c | 28 +--- drivers/usb/serial/generic.c | 29 + drivers/usb/serial/io_ti.c| 22 ++ drivers/usb/serial/ti_usb_3410_5052.c | 23 +++ drivers/usb/serial/usb-serial.c | 30 +- include/linux/usb/serial.h| 4 6 files changed, 92 insertions(+), 44 deletions(-) -- 1.8.2.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/
[PATCH 1/7] USB: serial: add wait_until_sent operation
Add wait_until_sent operation which can be used to wait for hardware
buffers to drain.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/usb-serial.c | 17 +
include/linux/usb/serial.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index cf75beb..31d2768 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -375,6 +375,22 @@ static int serial_chars_in_buffer(struct tty_struct *tty)
return count;
}
+static void serial_wait_until_sent(struct tty_struct *tty, int timeout)
+{
+ struct usb_serial_port *port = tty->driver_data;
+ struct usb_serial *serial = port->serial;
+
+ dev_dbg(tty->dev, "%s\n", __func__);
+
+ if (!port->serial->type->wait_until_sent)
+ return;
+
+ mutex_lock(&serial->disc_mutex);
+ if (!serial->disconnected)
+ port->serial->type->wait_until_sent(tty, timeout);
+ mutex_unlock(&serial->disc_mutex);
+}
+
static void serial_throttle(struct tty_struct *tty)
{
struct usb_serial_port *port = tty->driver_data;
@@ -1191,6 +1207,7 @@ static const struct tty_operations serial_ops = {
.unthrottle = serial_unthrottle,
.break_ctl =serial_break,
.chars_in_buffer = serial_chars_in_buffer,
+ .wait_until_sent = serial_wait_until_sent,
.tiocmget = serial_tiocmget,
.tiocmset = serial_tiocmset,
.get_icount = serial_get_icount,
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index b9b0f7b4..afbb7ee 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -268,6 +268,7 @@ struct usb_serial_driver {
struct usb_serial_port *port, struct ktermios *old);
void (*break_ctl)(struct tty_struct *tty, int break_state);
int (*chars_in_buffer)(struct tty_struct *tty);
+ void (*wait_until_sent)(struct tty_struct *tty, long timeout);
void (*throttle)(struct tty_struct *tty);
void (*unthrottle)(struct tty_struct *tty);
int (*tiocmget)(struct tty_struct *tty);
--
1.8.2.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/
[PATCH 2/7] USB: serial: add generic wait_until_sent implementation
Add generic wait_until_sent implementation which polls for empty
hardware buffers using the new port-operation tx_empty.
The generic implementation will be used for all sub-drivers that
implement tx_empty but does not define wait_until_sent.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/generic.c| 29 +
drivers/usb/serial/usb-serial.c | 2 ++
include/linux/usb/serial.h | 3 +++
3 files changed, 34 insertions(+)
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 297665f..70ae019 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -253,6 +253,35 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct
*tty)
}
EXPORT_SYMBOL_GPL(usb_serial_generic_chars_in_buffer);
+void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout)
+{
+ struct usb_serial_port *port = tty->driver_data;
+ unsigned int bps;
+ unsigned long period;
+ unsigned long expire;
+
+ /*
+* Use a poll-period of roughly the time it takes to send one
+* character or at least one jiffy.
+*/
+ bps = tty_get_baud_rate(tty);
+ period = max_t(unsigned long, (10 * HZ / bps), 1);
+ period = min_t(unsigned long, period, timeout);
+
+ dev_dbg(&port->dev, "%s - timeout = %u ms, period = %u ms\n",
+ __func__, jiffies_to_msecs(timeout),
+ jiffies_to_msecs(period));
+ expire = jiffies + timeout;
+ while (!port->serial->type->tx_empty(port)) {
+ schedule_timeout_interruptible(period);
+ if (signal_pending(current))
+ break;
+ if (time_after(jiffies, expire))
+ break;
+ }
+}
+EXPORT_SYMBOL_GPL(usb_serial_generic_wait_until_sent);
+
static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
int index, gfp_t mem_flags)
{
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 31d2768..60caf9c 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1333,6 +1333,8 @@ static void usb_serial_operations_init(struct
usb_serial_driver *device)
set_to_generic_if_null(device, close);
set_to_generic_if_null(device, write_room);
set_to_generic_if_null(device, chars_in_buffer);
+ if (device->tx_empty)
+ set_to_generic_if_null(device, wait_until_sent);
set_to_generic_if_null(device, read_bulk_callback);
set_to_generic_if_null(device, write_bulk_callback);
set_to_generic_if_null(device, process_read_urb);
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index afbb7ee..302ddf5 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -269,6 +269,7 @@ struct usb_serial_driver {
void (*break_ctl)(struct tty_struct *tty, int break_state);
int (*chars_in_buffer)(struct tty_struct *tty);
void (*wait_until_sent)(struct tty_struct *tty, long timeout);
+ bool (*tx_empty)(struct usb_serial_port *port);
void (*throttle)(struct tty_struct *tty);
void (*unthrottle)(struct tty_struct *tty);
int (*tiocmget)(struct tty_struct *tty);
@@ -328,6 +329,8 @@ extern void usb_serial_generic_close(struct usb_serial_port
*port);
extern int usb_serial_generic_resume(struct usb_serial *serial);
extern int usb_serial_generic_write_room(struct tty_struct *tty);
extern int usb_serial_generic_chars_in_buffer(struct tty_struct *tty);
+extern void usb_serial_generic_wait_until_sent(struct tty_struct *tty,
+ long timeout);
extern void usb_serial_generic_read_bulk_callback(struct urb *urb);
extern void usb_serial_generic_write_bulk_callback(struct urb *urb);
extern void usb_serial_generic_throttle(struct tty_struct *tty);
--
1.8.2.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/
[PATCH 5/7] USB: io_ti: fix chars_in_buffer overhead
Use the new generic usb-serial wait_until_sent implementation to wait
for hardware buffers to drain.
This removes the need to check the hardware buffers in chars_in_buffer
and thus removes the overhead introduced by commit 263e1f9f ("USB:
io_ti: query hardware-buffer status in chars_in_buffer") without
breaking tty_wait_until_sent (used by, for example, tcdrain, tcsendbreak
and close).
Signed-off-by: Johan Hovold
---
drivers/usb/serial/io_ti.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 158bf4b..1be6ba7 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -2019,8 +2019,6 @@ static int edge_chars_in_buffer(struct tty_struct *tty)
struct edgeport_port *edge_port = usb_get_serial_port_data(port);
int chars = 0;
unsigned long flags;
- int ret;
-
if (edge_port == NULL)
return 0;
@@ -2028,16 +2026,22 @@ static int edge_chars_in_buffer(struct tty_struct *tty)
chars = kfifo_len(&edge_port->write_fifo);
spin_unlock_irqrestore(&edge_port->ep_lock, flags);
- if (!chars) {
- ret = tx_active(edge_port);
- if (ret > 0)
- chars = ret;
- }
-
dev_dbg(&port->dev, "%s - returns %d\n", __func__, chars);
return chars;
}
+static bool edge_tx_empty(struct usb_serial_port *port)
+{
+ struct edgeport_port *edge_port = usb_get_serial_port_data(port);
+ int ret;
+
+ ret = tx_active(edge_port);
+ if (ret > 0)
+ return false;
+
+ return true;
+}
+
static void edge_throttle(struct tty_struct *tty)
{
struct usb_serial_port *port = tty->driver_data;
@@ -2557,6 +2561,7 @@ static struct usb_serial_driver edgeport_1port_device = {
.write = edge_write,
.write_room = edge_write_room,
.chars_in_buffer= edge_chars_in_buffer,
+ .tx_empty = edge_tx_empty,
.break_ctl = edge_break,
.read_int_callback = edge_interrupt_callback,
.read_bulk_callback = edge_bulk_in_callback,
@@ -2589,6 +2594,7 @@ static struct usb_serial_driver edgeport_2port_device = {
.write = edge_write,
.write_room = edge_write_room,
.chars_in_buffer= edge_chars_in_buffer,
+ .tx_empty = edge_tx_empty,
.break_ctl = edge_break,
.read_int_callback = edge_interrupt_callback,
.read_bulk_callback = edge_bulk_in_callback,
--
1.8.2.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/
[PATCH 7/7] USB: serial: clean up chars_in_buffer
No need to grab disconnect mutex in chars_in_buffer now that no
sub-driver is or should be querying hardware buffers anymore. (They
should use wait_until_sent.)
Signed-off-by: Johan Hovold
---
drivers/usb/serial/usb-serial.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 60caf9c..4753c00 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -359,20 +359,13 @@ static int serial_chars_in_buffer(struct tty_struct *tty)
{
struct usb_serial_port *port = tty->driver_data;
struct usb_serial *serial = port->serial;
- int count = 0;
dev_dbg(tty->dev, "%s\n", __func__);
- mutex_lock(&serial->disc_mutex);
- /* if the device was unplugged then any remaining characters
- fell out of the connector ;) */
if (serial->disconnected)
- count = 0;
- else
- count = serial->type->chars_in_buffer(tty);
- mutex_unlock(&serial->disc_mutex);
+ return 0;
- return count;
+ return serial->type->chars_in_buffer(tty);
}
static void serial_wait_until_sent(struct tty_struct *tty, int timeout)
--
1.8.2.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/
[PATCH 4/7] USB: ftdi_sio: fix chars_in_buffer overhead
Use the new generic usb-serial wait_until_sent implementation to wait
for hardware buffers to drain.
This removes the need to check the hardware buffers in chars_in_buffer
and thus removes the overhead introduced by commit 6f602912 ("usb:
serial: ftdi_sio: Add missing chars_in_buffer function") without
breaking tty_wait_until_sent (used by, for example, tcdrain, tcsendbreak
and close).
Reported-by: Stas Sergeev
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 19 +--
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 1159fd4..a62a75a 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -924,7 +924,7 @@ static int ftdi_tiocmset(struct tty_struct *tty,
static int ftdi_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg);
static void ftdi_break_ctl(struct tty_struct *tty, int break_state);
-static int ftdi_chars_in_buffer(struct tty_struct *tty);
+static bool ftdi_tx_empty(struct usb_serial_port *port);
static int ftdi_get_modem_status(struct usb_serial_port *port,
unsigned char status[2]);
@@ -961,7 +961,7 @@ static struct usb_serial_driver ftdi_sio_device = {
.ioctl =ftdi_ioctl,
.set_termios = ftdi_set_termios,
.break_ctl =ftdi_break_ctl,
- .chars_in_buffer = ftdi_chars_in_buffer,
+ .tx_empty = ftdi_tx_empty,
};
static struct usb_serial_driver * const serial_drivers[] = {
@@ -2056,27 +2056,18 @@ static void ftdi_break_ctl(struct tty_struct *tty, int
break_state)
}
-static int ftdi_chars_in_buffer(struct tty_struct *tty)
+static bool ftdi_tx_empty(struct usb_serial_port *port)
{
- struct usb_serial_port *port = tty->driver_data;
- int chars;
unsigned char buf[2];
int ret;
- chars = usb_serial_generic_chars_in_buffer(tty);
- if (chars)
- goto out;
-
- /* Check if hardware buffer is empty. */
ret = ftdi_get_modem_status(port, buf);
if (ret == 2) {
if (!(buf[1] & FTDI_RS_TEMT))
- chars = 1;
+ return false;
}
-out:
- dev_dbg(&port->dev, "%s - %d\n", __func__, chars);
- return chars;
+ return true;
}
/* old_termios contains the original termios settings and tty->termios contains
--
1.8.2.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/
Re: [PATCH 2/7] USB: serial: add generic wait_until_sent implementation
On Wed, May 08, 2013 at 06:25:13PM +0400, Stas Sergeev wrote: > 05.05.2013 22:32, Johan Hovold пишет: > > Add generic wait_until_sent implementation which polls for empty > > hardware buffers using the new port-operation tx_empty. > > > > The generic implementation will be used for all sub-drivers that > > implement tx_empty but does not define wait_until_sent. > Hi Johan. > > The customer reports an Oops below. > Does that ring any bells? > Well, there is only one division in usb_serial_generic_wait_until_sent(), > anyway. :) Yes, you're right I forgot about B0. I'll send a v2. Thanks for catching this, Johan -- 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/
[PATCH v2 2/8] USB: serial: add generic wait_until_sent implementation
Add generic wait_until_sent implementation which polls for empty
hardware buffers using the new port-operation tx_empty.
The generic implementation will be used for all sub-drivers that
implement tx_empty but does not define wait_until_sent.
Signed-off-by: Johan Hovold
---
v2: make sure to handle B0
drivers/usb/serial/generic.c| 31 +++
drivers/usb/serial/usb-serial.c | 2 ++
include/linux/usb/serial.h | 3 +++
3 files changed, 36 insertions(+)
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 297665f..ba45170 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -253,6 +253,37 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct
*tty)
}
EXPORT_SYMBOL_GPL(usb_serial_generic_chars_in_buffer);
+void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout)
+{
+ struct usb_serial_port *port = tty->driver_data;
+ unsigned int bps;
+ unsigned long period;
+ unsigned long expire;
+
+ bps = tty_get_baud_rate(tty);
+ if (!bps)
+ bps = 9600; /* B0 */
+ /*
+* Use a poll-period of roughly the time it takes to send one
+* character or at least one jiffy.
+*/
+ period = max_t(unsigned long, (10 * HZ / bps), 1);
+ period = min_t(unsigned long, period, timeout);
+
+ dev_dbg(&port->dev, "%s - timeout = %u ms, period = %u ms\n",
+ __func__, jiffies_to_msecs(timeout),
+ jiffies_to_msecs(period));
+ expire = jiffies + timeout;
+ while (!port->serial->type->tx_empty(port)) {
+ schedule_timeout_interruptible(period);
+ if (signal_pending(current))
+ break;
+ if (time_after(jiffies, expire))
+ break;
+ }
+}
+EXPORT_SYMBOL_GPL(usb_serial_generic_wait_until_sent);
+
static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
int index, gfp_t mem_flags)
{
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 31d2768..60caf9c 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1333,6 +1333,8 @@ static void usb_serial_operations_init(struct
usb_serial_driver *device)
set_to_generic_if_null(device, close);
set_to_generic_if_null(device, write_room);
set_to_generic_if_null(device, chars_in_buffer);
+ if (device->tx_empty)
+ set_to_generic_if_null(device, wait_until_sent);
set_to_generic_if_null(device, read_bulk_callback);
set_to_generic_if_null(device, write_bulk_callback);
set_to_generic_if_null(device, process_read_urb);
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index afbb7ee..302ddf5 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -269,6 +269,7 @@ struct usb_serial_driver {
void (*break_ctl)(struct tty_struct *tty, int break_state);
int (*chars_in_buffer)(struct tty_struct *tty);
void (*wait_until_sent)(struct tty_struct *tty, long timeout);
+ bool (*tx_empty)(struct usb_serial_port *port);
void (*throttle)(struct tty_struct *tty);
void (*unthrottle)(struct tty_struct *tty);
int (*tiocmget)(struct tty_struct *tty);
@@ -328,6 +329,8 @@ extern void usb_serial_generic_close(struct usb_serial_port
*port);
extern int usb_serial_generic_resume(struct usb_serial *serial);
extern int usb_serial_generic_write_room(struct tty_struct *tty);
extern int usb_serial_generic_chars_in_buffer(struct tty_struct *tty);
+extern void usb_serial_generic_wait_until_sent(struct tty_struct *tty,
+ long timeout);
extern void usb_serial_generic_read_bulk_callback(struct urb *urb);
extern void usb_serial_generic_write_bulk_callback(struct urb *urb);
extern void usb_serial_generic_throttle(struct tty_struct *tty);
--
1.8.2.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/
[PATCH] rtc: rtc-at91rm9200: fix missing iounmap
Add missing iounmap to probe error path and remove.
Cc: stable
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 434ebc3..eebd8ac 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -297,7 +297,7 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
"at91_rtc", pdev);
if (ret) {
dev_err(&pdev->dev, "IRQ %d already in use.\n", irq);
- return ret;
+ goto err_unmap;
}
/* cpu init code should really have flagged this device as
@@ -309,13 +309,20 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
rtc = rtc_device_register(pdev->name, &pdev->dev,
&at91_rtc_ops, THIS_MODULE);
if (IS_ERR(rtc)) {
- free_irq(irq, pdev);
- return PTR_ERR(rtc);
+ ret = PTR_ERR(rtc);
+ goto err_free_irq;
}
platform_set_drvdata(pdev, rtc);
dev_info(&pdev->dev, "AT91 Real Time Clock driver.\n");
return 0;
+
+err_free_irq:
+ free_irq(irq, pdev);
+err_unmap:
+ iounmap(at91_rtc_regs);
+
+ return ret;
}
/*
@@ -332,6 +339,7 @@ static int __exit at91_rtc_remove(struct platform_device
*pdev)
free_irq(irq, pdev);
rtc_device_unregister(rtc);
+ iounmap(at91_rtc_regs);
platform_set_drvdata(pdev, NULL);
return 0;
--
1.8.1.5
--
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/
Re: [PATCH v2 0/5] ARM: at91: fix hanged boot
On Mon, Mar 11, 2013 at 07:07:54PM +0100, Johan Hovold wrote: > These patches fix a few severe issues affecting most AT91 SOCs where > boot can hang after a non-general reset, and where the only way to get > the system booting again is to do a general reset -- something which > could require physically removing any backup battery. Have you had time to look at these patches yet, Nicolas? I don't think not having decided on the path forward for DT-support for rtc-at91sam9 needs to be a blocker. The rtt-nodes will be needed in any case. I could respin the series on top of the DT-patch for rtc-at91rm9200, and add interrupt and status-disabled properties to the DT-nodes as well. Thanks, Johan > The problems stem from the fact that the RTC and RTT-peripherals are > powered by backup power (VDDBU) and are not reset on wake-up, user, > watchdog or software reset. Consequently, RTC and RTT-alarms and their > interrupts may be enabled at boot, leading to a system lock-up when an > interrupt arrives on the shared system-interrupt line before the > appropriate handler (e.g. RTC-driver) has been installed. > > The easiest way to trigger this is to simply wake up from an RTC-alarm > on at91sam9g45. The RTC-driver currently does not disable interrupts at > shutdown so even after a clean shut-down the system will always hang > after waking up. > > The first patch fixes this very general case of RTC-wake up after a > clean shutdown in the RTC-driver and is marked for stable as it is > perfectly straight-forward. [ Note that the other, RTT-based, AT91 > RTC-driver already disables its interrupts at shutdown. ] > > The more general problem can be triggered, for example, by doing a > user-reset while updating the RTC-time or if an RTC or RTT-alarm goes > off after a non-clean shutdown. > > To fix this I propose that arch-code should mask the relevant interrupts > before enabling the system interrupt at early boot, and this is what > the fifth patch does. To access the RTC-registers I choose to revert a > recent patch that moved the register definitions to drivers/rtc. > > Arguably, the relevant interrupts could also be disabled in bootloaders, > but I suggest fixing it in the kernel once and for all. > > The patches have been tested on at91sam9263 and at91sam9g45 (non-DT), > and compile-tested for the other SOCs and DT. > > Johan > > > v2: > - add DT-support > - make sys_irq_mask non-mandatory > > > Johan Hovold (5): > ARM: at91/rtc: fix boot after RTC wake-up > ARM: at91/dts: add RTC nodes > ARM: at91/dts: add RTT nodes > Revert "arm: at91: move at91rm9200 rtc header in drivers/rtc" > ARM: at91: fix hanged boot -- 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/
[PATCH 2/4] TTY: synclink_gt: fix DTR being raised on hang up
Make sure to check ASYNC_INITIALISED before raising DTR when waking up
from blocked open in block_til_ready.
Currently DTR could get raised at hang up as a blocked process would
raise DTR unconditionally before checking for hang up and returning.
Signed-off-by: Johan Hovold
---
drivers/tty/synclink_gt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index aa9eece..1abf946 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -3308,7 +3308,7 @@ static int block_til_ready(struct tty_struct *tty, struct
file *filp,
port->blocked_open++;
while (1) {
- if ((tty->termios.c_cflag & CBAUD))
+ if (C_BAUD(tty) && test_bit(ASYNCB_INITIALIZED, &port->flags))
tty_port_raise_dtr_rts(port);
set_current_state(TASK_INTERRUPTIBLE);
--
1.8.1.5
--
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/
[PATCH 3/4] TTY: synclinkmp: fix DTR being raised on hang up
Make sure to check ASYNC_INITIALISED before raising DTR when waking up
from blocked open in block_til_ready.
Currently DTR could get raised at hang up as a blocked process would
raise DTR unconditionally before checking for hang up and returning.
Signed-off-by: Johan Hovold
---
drivers/tty/synclinkmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/synclinkmp.c b/drivers/tty/synclinkmp.c
index 6d5780c..ff17138 100644
--- a/drivers/tty/synclinkmp.c
+++ b/drivers/tty/synclinkmp.c
@@ -3329,7 +3329,7 @@ static int block_til_ready(struct tty_struct *tty, struct
file *filp,
port->blocked_open++;
while (1) {
- if (tty->termios.c_cflag & CBAUD)
+ if (C_BAUD(tty) && test_bit(ASYNCB_INITIALIZED, &port->flags))
tty_port_raise_dtr_rts(port);
set_current_state(TASK_INTERRUPTIBLE);
--
1.8.1.5
--
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/
[PATCH 4/4] TTY: ircomm: fix DTR being raised on hang up
Make sure to check ASYNC_INITIALISED before raising DTR when waking up
from blocked open in ircomm_tty_block_til_ready.
Currently DTR could get raised at hang up as a blocked process would
raise DTR unconditionally before checking for hang up and returning.
Cc: David S. Miller
Signed-off-by: Johan Hovold
---
net/irda/ircomm/ircomm_tty.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 362ba47..41ac7938 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -328,7 +328,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb
*self,
spin_unlock_irqrestore(&port->lock, flags);
while (1) {
- if (tty->termios.c_cflag & CBAUD)
+ if (C_BAUD(tty) && test_bit(ASYNCB_INITIALIZED, &port->flags))
tty_port_raise_dtr_rts(port);
set_current_state(TASK_INTERRUPTIBLE);
--
1.8.1.5
--
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/
[PATCH 1/4] TTY: synclink: fix DTR being raised on hang up
Make sure to check ASYNC_INITIALISED before raising DTR when waking up
from blocked open in block_til_ready.
Currently DTR could get raised at hang up as a blocked process would
raise DTR unconditionally before checking for hang up and returning.
Signed-off-by: Johan Hovold
---
drivers/tty/synclink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/synclink.c b/drivers/tty/synclink.c
index 72d6071..8eaf1ab 100644
--- a/drivers/tty/synclink.c
+++ b/drivers/tty/synclink.c
@@ -3308,7 +3308,7 @@ static int block_til_ready(struct tty_struct *tty, struct
file * filp,
port->blocked_open++;
while (1) {
- if (tty->termios.c_cflag & CBAUD)
+ if (C_BAUD(tty) && test_bit(ASYNCB_INITIALIZED, &port->flags))
tty_port_raise_dtr_rts(port);
set_current_state(TASK_INTERRUPTIBLE);
--
1.8.1.5
--
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/
[PATCH 0/4] TTY: fix DTR being raised on hang up
These patches fix four custom block_til_ready implementations which
could raise DTR after first having dropped it at hangup.
This was fixed in the tty-port implementation by commit e584a02cf ("TTY:
fix DTR being raised on hang up") in tty-next.
Note that the crisv10-driver still suffers from this behaviour but is
broken in other ways as it, for example, does not honour CBAUD or raise
DTR at non-blocking open.
Thanks,
Johan
Johan Hovold (4):
TTY: synclink: fix DTR being raised on hang up
TTY: synclink_gt: fix DTR being raised on hang up
TTY: synclinkmp: fix DTR being raised on hang up
TTY: ircomm: fix DTR being raised on hang up
drivers/tty/synclink.c | 2 +-
drivers/tty/synclink_gt.c| 2 +-
drivers/tty/synclinkmp.c | 2 +-
net/irda/ircomm/ircomm_tty.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
--
1.8.1.5
--
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/
Re: [rtc-linux] Re: [PATCH v2 0/5] ARM: at91: fix hanged boot
On Thu, Apr 11, 2013 at 06:54:14PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 17:55 Thu 11 Apr , Johan Hovold wrote: > > On Mon, Mar 11, 2013 at 07:07:54PM +0100, Johan Hovold wrote: > > > These patches fix a few severe issues affecting most AT91 SOCs where > > > boot can hang after a non-general reset, and where the only way to get > > > the system booting again is to do a general reset -- something which > > > could require physically removing any backup battery. > > > > Have you had time to look at these patches yet, Nicolas? > > > > I don't think not having decided on the path forward for DT-support for > > rtc-at91sam9 needs to be a blocker. The rtt-nodes will be needed in any > > case. > > > > I could respin the series on top of the DT-patch for rtc-at91rm9200, and > > add interrupt and status-disabled properties to the DT-nodes as well. > for this this is still a no go > > this way too much ugly I understand that you prefer fixing every bootloader. I was just making sure everyone agrees that that is the best solution. The two interrupt masks has to be cleared before the kernel enables the system interrupt; either it needs to be done by the bootloader or by the at91 arch code. The various bootloaders may not know anything about RTT or RTC, but have all made sure interrupts are disabled before executing the kernel. That is, they have fulfilled the requirement that interrupts must be disabled. So the trade-off seems to be: Either we fix this once and for all using the infrastructure already in place in the kernel (DT), or risk further (apparently) bricked systems as there are bound to be bootloaders that won't get updated. [...] > > > The problems stem from the fact that the RTC and RTT-peripherals are > > > powered by backup power (VDDBU) and are not reset on wake-up, user, > > > watchdog or software reset. Consequently, RTC and RTT-alarms and their > > > interrupts may be enabled at boot, leading to a system lock-up when an > > > interrupt arrives on the shared system-interrupt line before the > > > appropriate handler (e.g. RTC-driver) has been installed. > > > > > > The easiest way to trigger this is to simply wake up from an RTC-alarm > > > on at91sam9g45. The RTC-driver currently does not disable interrupts at > > > shutdown so even after a clean shut-down the system will always hang > > > after waking up. > > > > > > The first patch fixes this very general case of RTC-wake up after a > > > clean shutdown in the RTC-driver and is marked for stable as it is > > > perfectly straight-forward. [ Note that the other, RTT-based, AT91 > > > RTC-driver already disables its interrupts at shutdown. ] And what about this patch? If it's decided that every bootloader needs to be updated, then perhaps it's better to risk bricked systems also after a clean shutdown to enforce those updates? Should we then remove the corresponding disable of interrupts at shutdown from the rtc-at91sam9 driver by the same logic? > > > The more general problem can be triggered, for example, by doing a > > > user-reset while updating the RTC-time or if an RTC or RTT-alarm goes > > > off after a non-clean shutdown. > > > > > > To fix this I propose that arch-code should mask the relevant interrupts > > > before enabling the system interrupt at early boot, and this is what > > > the fifth patch does. To access the RTC-registers I choose to revert a > > > recent patch that moved the register definitions to drivers/rtc. > > > > > > Arguably, the relevant interrupts could also be disabled in bootloaders, > > > but I suggest fixing it in the kernel once and for all. Thanks, Johan -- 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/
Re: [PATCH 0/7] USB: serial: add wait_until_sent-support
On Fri, May 17, 2013 at 10:46:37AM -0500, Caylan Larson wrote: > Johan, > > I have tested these patches and the performance is much better. Thank you. Thanks for testing. The patches are already in the usb tree (usb-linus branch) and should show up in v3.10-rc soon. Johan > Tested-by: Caylan Larson > > Caylan > > > On May 5, 2013, at 1:32 PM, Johan Hovold wrote: > > > These patches add wait_until_sent-support to usb-serial, which removes > > the need to check hardware buffers in chars_in_buffer. > > > > This fixes a problem in ftdi_sio (since 3.7) where select or TIOCMOUTQ > > would take much longer than before due the hardware buffers being > > queried. > > > > Hardware buffers are also currently checked in chars_in_buffer in io_ti > > (since 3.8) and ti_usb_3410_5052 (in 3.10). > > > > Note that simply removing the hardware-buffer checks (e.g. for the > > stable trees) would break tty_wait_until_sent, which is used, for > > instance, by tcdrain, tcsendbreak, and close. -- 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/
[PATCH v3 5/5] rtc-at91rm9200: use shadow IMR on at91sam9x5
Add support for the at91sam9x5-family which must use the shadow
interrupt mask due to a hardware issue (causing RTC_IMR to always be
zero).
Signed-off-by: Johan Hovold
---
Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt | 2 +-
arch/arm/boot/dts/at91sam9x5.dtsi | 2 +-
drivers/rtc/rtc-at91rm9200.c | 7 +++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
index 2a3feab..34c1505 100644
--- a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
@@ -1,7 +1,7 @@
Atmel AT91RM9200 Real Time Clock
Required properties:
-- compatible: should be: "atmel,at91rm9200-rtc"
+- compatible: should be: "atmel,at91rm9200-rtc" or "atmel,at91sam9x5-rtc"
- reg: physical base address of the controller and length of memory mapped
region.
- interrupts: rtc alarm/event interrupt
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi
b/arch/arm/boot/dts/at91sam9x5.dtsi
index 1145ac3..b5833d1f 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -643,7 +643,7 @@
};
rtc@feb0 {
- compatible = "atmel,at91rm9200-rtc";
+ compatible = "atmel,at91sam9x5-rtc";
reg = <0xfeb0 0x40>;
interrupts = <1 4 7>;
status = "disabled";
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 205701e..47416e9 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -309,12 +309,19 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
*dev_id)
static const struct at91_rtc_config at91rm9200_config = {
};
+static const struct at91_rtc_config at91sam9x5_config = {
+ .use_shadow_imr = true,
+};
+
#ifdef CONFIG_OF
static const struct of_device_id at91_rtc_dt_ids[] = {
{
.compatible = "atmel,at91rm9200-rtc",
.data = &at91rm9200_config,
}, {
+ .compatible = "atmel,at91sam9x5-rtc",
+ .data = &at91sam9x5_config,
+ }, {
/* sentinel */
}
};
--
1.8.2.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/
[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask
This is an update of the shadow-interrupt-mask series against v3.10-rc2. I guess we need Atmel to confirm that all sam9x5 SoCs are indeed affected. If not, then some probing mechanism as the one Doug suggested could be implemented on top of (a subset of) these patches. What do you say, Nicolas? Note that the first patch (adding a missing OF compile guard) could be applied straight away. Thanks, Johan v3: - rebase against v3.10-rc2 - remove some comments v2: - rebase on top of DT-support patch by Joachim Eastwood - add missing brace in DT-id table Johan Hovold (5): rtc-at91rm9200: add match-table compile guard rtc-at91rm9200: add configuration support rtc-at91rm9200: refactor interrupt-register handling rtc-at91rm9200: add shadow interrupt mask rtc-at91rm9200: use shadow IMR on at91sam9x5 .../bindings/rtc/atmel,at91rm9200-rtc.txt | 2 +- arch/arm/boot/dts/at91sam9x5.dtsi | 2 +- drivers/rtc/rtc-at91rm9200.c | 131 + 3 files changed, 113 insertions(+), 22 deletions(-) -- 1.8.2.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/
[PATCH v3 2/5] rtc-at91rm9200: add configuration support
Add configuration support which can be used to implement SoC-specific
workarounds for broken hardware.
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 46
1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index eeeb73f..ab2024b 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -42,6 +42,10 @@
#define AT91_RTC_EPOCH 1900UL /* just like arch/arm/common/rtctime.c
*/
+struct at91_rtc_config {
+};
+
+static const struct at91_rtc_config *at91_rtc_config;
static DECLARE_COMPLETION(at91_rtc_updated);
static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
@@ -250,6 +254,36 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
*dev_id)
return IRQ_NONE;/* not handled */
}
+static const struct at91_rtc_config at91rm9200_config = {
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id at91_rtc_dt_ids[] = {
+ {
+ .compatible = "atmel,at91rm9200-rtc",
+ .data = &at91rm9200_config,
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
+#endif
+
+static const struct at91_rtc_config *
+at91_rtc_get_config(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+
+ if (pdev->dev.of_node) {
+ match = of_match_node(at91_rtc_dt_ids, pdev->dev.of_node);
+ if (!match)
+ return NULL;
+ return (const struct at91_rtc_config *)match->data;
+ }
+
+ return &at91rm9200_config;
+}
+
static const struct rtc_class_ops at91_rtc_ops = {
.read_time = at91_rtc_readtime,
.set_time = at91_rtc_settime,
@@ -268,6 +302,10 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
struct resource *regs;
int ret = 0;
+ at91_rtc_config = at91_rtc_get_config(pdev);
+ if (!at91_rtc_config)
+ return -ENODEV;
+
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
dev_err(&pdev->dev, "no mmio resource defined\n");
@@ -383,14 +421,6 @@ static int at91_rtc_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(at91_rtc_pm_ops, at91_rtc_suspend, at91_rtc_resume);
-#ifdef CONFIG_OF
-static const struct of_device_id at91_rtc_dt_ids[] = {
- { .compatible = "atmel,at91rm9200-rtc" },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
-#endif
-
static struct platform_driver at91_rtc_driver = {
.remove = __exit_p(at91_rtc_remove),
.driver = {
--
1.8.2.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/
[PATCH v3 4/5] rtc-at91rm9200: add shadow interrupt mask
Add shadow interrupt-mask register which can be used on SoCs where the
actual hardware register is broken.
Note that some care needs to be taken to make sure the shadow mask
corresponds to the actual hardware state. The added overhead is not an
issue for the non-broken SoCs due to the relatively infrequent
interrupt-mask updates. We do, however, only use the shadow mask value
as a fall-back when it actually needed as there is still a theoretical
possibility that the mask is incorrect (see the code for details).
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 39 ++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 9592d08..205701e 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
@@ -43,6 +44,7 @@
#define AT91_RTC_EPOCH 1900UL /* just like arch/arm/common/rtctime.c
*/
struct at91_rtc_config {
+ bool use_shadow_imr;
};
static const struct at91_rtc_config *at91_rtc_config;
@@ -50,20 +52,55 @@ static DECLARE_COMPLETION(at91_rtc_updated);
static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
static int irq;
+static DEFINE_SPINLOCK(at91_rtc_lock);
+static u32 at91_rtc_shadow_imr;
static void at91_rtc_write_ier(u32 mask)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&at91_rtc_lock, flags);
+ at91_rtc_shadow_imr |= mask;
at91_rtc_write(AT91_RTC_IER, mask);
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
}
static void at91_rtc_write_idr(u32 mask)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&at91_rtc_lock, flags);
at91_rtc_write(AT91_RTC_IDR, mask);
+ /*
+* Register read back (of any RTC-register) needed to make sure
+* IDR-register write has reached the peripheral before updating
+* shadow mask.
+*
+* Note that there is still a possibility that the mask is updated
+* before interrupts have actually been disabled in hardware. The only
+* way to be certain would be to poll the IMR-register, which is is
+* the very register we are trying to emulate. The register read back
+* is a reasonable heuristic.
+*/
+ at91_rtc_read(AT91_RTC_SR);
+ at91_rtc_shadow_imr &= ~mask;
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
}
static u32 at91_rtc_read_imr(void)
{
- return at91_rtc_read(AT91_RTC_IMR);
+ unsigned long flags;
+ u32 mask;
+
+ if (at91_rtc_config->use_shadow_imr) {
+ spin_lock_irqsave(&at91_rtc_lock, flags);
+ mask = at91_rtc_shadow_imr;
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
+ } else {
+ mask = at91_rtc_read(AT91_RTC_IMR);
+ }
+
+ return mask;
}
/*
--
1.8.2.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/
[PATCH v3 1/5] rtc-at91rm9200: add match-table compile guard
Add missing match-table compile guard.
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 0eab77b..eeeb73f 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -383,11 +383,13 @@ static int at91_rtc_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(at91_rtc_pm_ops, at91_rtc_suspend, at91_rtc_resume);
+#ifdef CONFIG_OF
static const struct of_device_id at91_rtc_dt_ids[] = {
{ .compatible = "atmel,at91rm9200-rtc" },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
+#endif
static struct platform_driver at91_rtc_driver = {
.remove = __exit_p(at91_rtc_remove),
--
1.8.2.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/
[PATCH v3 3/5] rtc-at91rm9200: refactor interrupt-register handling
Add accessors for the interrupt register.
This will allow us to easily add a shadow interrupt-mask register to
use on SoCs where the interrupt-mask register cannot be used.
Signed-off-by: Johan Hovold
---
drivers/rtc/rtc-at91rm9200.c | 43 +--
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index ab2024b..9592d08 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -51,6 +51,21 @@ static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
static int irq;
+static void at91_rtc_write_ier(u32 mask)
+{
+ at91_rtc_write(AT91_RTC_IER, mask);
+}
+
+static void at91_rtc_write_idr(u32 mask)
+{
+ at91_rtc_write(AT91_RTC_IDR, mask);
+}
+
+static u32 at91_rtc_read_imr(void)
+{
+ return at91_rtc_read(AT91_RTC_IMR);
+}
+
/*
* Decode time/date into rtc_time structure
*/
@@ -114,9 +129,9 @@ static int at91_rtc_settime(struct device *dev, struct
rtc_time *tm)
cr = at91_rtc_read(AT91_RTC_CR);
at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | AT91_RTC_UPDTIM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ACKUPD);
+ at91_rtc_write_ier(AT91_RTC_ACKUPD);
wait_for_completion(&at91_rtc_updated); /* wait for ACKUPD interrupt */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD);
+ at91_rtc_write_idr(AT91_RTC_ACKUPD);
at91_rtc_write(AT91_RTC_TIMR,
bin2bcd(tm->tm_sec) << 0
@@ -148,7 +163,7 @@ static int at91_rtc_readalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
tm->tm_year = at91_alarm_year - 1900;
- alrm->enabled = (at91_rtc_read(AT91_RTC_IMR) & AT91_RTC_ALARM)
+ alrm->enabled = (at91_rtc_read_imr() & AT91_RTC_ALARM)
? 1 : 0;
dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -173,7 +188,7 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm.tm_min = alrm->time.tm_min;
tm.tm_sec = alrm->time.tm_sec;
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+ at91_rtc_write_idr(AT91_RTC_ALARM);
at91_rtc_write(AT91_RTC_TIMALR,
bin2bcd(tm.tm_sec) << 0
| bin2bcd(tm.tm_min) << 8
@@ -186,7 +201,7 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
if (alrm->enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+ at91_rtc_write_ier(AT91_RTC_ALARM);
}
dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -202,9 +217,9 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
if (enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+ at91_rtc_write_ier(AT91_RTC_ALARM);
} else
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+ at91_rtc_write_idr(AT91_RTC_ALARM);
return 0;
}
@@ -213,7 +228,7 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
*/
static int at91_rtc_proc(struct device *dev, struct seq_file *seq)
{
- unsigned long imr = at91_rtc_read(AT91_RTC_IMR);
+ unsigned long imr = at91_rtc_read_imr();
seq_printf(seq, "update_IRQ\t: %s\n",
(imr & AT91_RTC_ACKUPD) ? "yes" : "no");
@@ -233,7 +248,7 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
unsigned int rtsr;
unsigned long events = 0;
- rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read(AT91_RTC_IMR);
+ rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read_imr();
if (rtsr) { /* this interrupt is shared! Is it ours? */
if (rtsr & AT91_RTC_ALARM)
events |= (RTC_AF | RTC_IRQF);
@@ -328,7 +343,7 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
at91_rtc_write(AT91_RTC_MR, 0); /* 24 hour mode */
/* Disable all interrupts */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+ at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
AT91_RTC_SECEV | AT91_RTC_TIMEV |
AT91_RTC_CALEV);
@@ -373,7 +388,7 @@ static int __exit at91_rtc_remove(struct platform_device
*pdev)
struct rtc_device *rtc = platform_get_drvdata(pdev);
/* Disable all interrupts */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+
Re: [PATCH] rtc: rtc-at91rm9200: use a variable for storing IMR
On Tue, Mar 26, 2013 at 05:09:59PM -0400, Douglas Gilbert wrote:
> On 13-03-26 03:27 PM, Johan Hovold wrote:
> > On Fri, Mar 15, 2013 at 06:37:12PM +0100, Nicolas Ferre wrote:
> >> On some revisions of AT91 SoCs, the RTC IMR register is not working.
> >> Instead of elaborating a workaround for that specific SoC or IP version,
> >> we simply use a software variable to store the Interrupt Mask Register and
> >> modify it for each enabling/disabling of an interrupt. The overhead of this
> >> is negligible anyway.
> >
> > The patch does not add any memory barriers or register read-backs when
> > manipulating the interrupt-mask variable. This could possibly lead to
> > spurious interrupts both when enabling and disabling the various
> > RTC-interrupts due to write reordering and bus latencies.
> >
> > Has this been considered? And is this reason enough for a more targeted
> > work-around so that the SOCs with functional RTC_IMR are not affected?
>
> The SoCs in question use a single embedded ARM926EJ-S and
> according to the Atmel documentation, that CPU's instruction
> set contains no barrier (or related) instructions.
The ARM926EJ-S actually does have a Drain Write Buffer instruction but
it's not used by the ARM barrier-implementation unless
CONFIG_ARM_DMA_MEM_BUFFERABLE or CONFIG_SMP is set.
However, wmb() always implies a compiler barrier which is what is needed
in this case.
> In the arch/arm/mach-at91 sub-tree of the kernel source
> I can find no use of the wmb() call. Also checked all drivers
> in the kernel containing "at91" and none called wmb().
I/O-operations are normally not reordered, but this patch is faking a
hardware register and thus extra care needs to be taken.
To repeat:
> @@ -198,9 +203,12 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
> unsigned int enabled)
>
> if (enabled) {
> at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
> + at91_rtc_imr |= AT91_RTC_ALARM;
Here a barrier is needed to prevent the compiler from reordering the two
writes (i.e., mask update and interrupt enable).
> at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
> - } else
> + } else {
> at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
Here a barrier is again needed to prevent the compiler from reordering,
but we also need a register read back (of some RTC-register) before
updating the mask. Without the register read back, there could be a
window where the mask does not match the hardware state due to bus
latencies.
Note that even with a register read back there is a (theoretical)
possibility that the interrupts have not yet been disabled when the fake
mask is updated. The only way to know for sure is to poll RTC_IMR but
that is the very register you're trying to emulate.
> + at91_rtc_imr &= ~AT91_RTC_ALARM;
> + }
>
> return 0;
> }
In the worst-case scenario ignoring the shared RTC-interrupt could lead
to the disabling of the system interrupt and thus also PIT, DBGU, ...
I think this patch should be reverted and a fix for the broken SoCs be
implemented which does not penalise the other SoCs. That is, only
fall-back to faking IMR on the SoCs where it is actually broken.
Nicolas, should I send a revert patch and follow up with a fix for the
broken SoCs which includes the required barriers and read-backs?
Note that the patch is already being picked up for some stable trees.
The fix I'm proposing would require adding minimal DT-support to the
driver and is not really stable material. Therefore, a revert followed
by a patch for 3.10 seems like the way to go.
Johan
--
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/
Re: [PATCH] rtc: rtc-at91rm9200: use a variable for storing IMR
On Thu, Mar 28, 2013 at 05:16:00PM +0100, Nicolas Ferre wrote:
> On 03/28/2013 10:57 AM, Johan Hovold :
> > On Tue, Mar 26, 2013 at 05:09:59PM -0400, Douglas Gilbert wrote:
> >> On 13-03-26 03:27 PM, Johan Hovold wrote:
> >>> On Fri, Mar 15, 2013 at 06:37:12PM +0100, Nicolas Ferre wrote:
[...]
> >> @@ -198,9 +203,12 @@ static int at91_rtc_alarm_irq_enable(struct device
> >> *dev, unsigned int enabled)
> >>
> >> if (enabled) {
> >> at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
> >> + at91_rtc_imr |= AT91_RTC_ALARM;
> >
> > Here a barrier is needed to prevent the compiler from reordering the two
> > writes (i.e., mask update and interrupt enable).
> >
> >> at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
> >> - } else
> >> + } else {
> >> at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
> >
> > Here a barrier is again needed to prevent the compiler from reordering,
> > but we also need a register read back (of some RTC-register) before
> > updating the mask. Without the register read back, there could be a
> > window where the mask does not match the hardware state due to bus
> > latencies.
> >
> > Note that even with a register read back there is a (theoretical)
> > possibility that the interrupts have not yet been disabled when the fake
> > mask is updated. The only way to know for sure is to poll RTC_IMR but
> > that is the very register you're trying to emulate.
>
> In fact, if we protect the two code lines with the proper spinlock, we
> may find that all this reordering issue will not lead to a race
> condition. So I guess it is a simpler solution to the problem that you
> highlight.
A spinlock would also be a solution to the reordering problem, albeit a
slightly heavier one than the wmb(). A comment from from Doug made me
realise that one more barrier would actually be needed, so I think using
a spinlock is indeed preferred.
It would however not be sufficient to address the second problem, which
is that the interrupt disable is not immediate. An RTC-register read
back is needed to make sure the IDR-write has reached the peripheral,
but as I mentioned above it is merely a reasonable heuristic as the only
way to be certain that the interrupts have been disabled in hardware
would be to poll the RTC_IMR-register, which is register we are trying
to emulate.
In practice, the read-back heuristic would most likely be perfectly
sufficient, but I'd prefer this to only be used for SoCs where the
RTC_IMR-register is actually broken.
> >> + at91_rtc_imr &= ~AT91_RTC_ALARM;
> >> + }
> >>
> >> return 0;
> >> }
> >
> > In the worst-case scenario ignoring the shared RTC-interrupt could lead
> > to the disabling of the system interrupt and thus also PIT, DBGU, ...
> >
> > I think this patch should be reverted and a fix for the broken SoCs be
> > implemented which does not penalise the other SoCs. That is, only
> > fall-back to faking IMR on the SoCs where it is actually broken.
> >
> > Nicolas, should I send a revert patch and follow up with a fix for the
> > broken SoCs which includes the required barriers and read-backs?
>
> I prefer to not distinguish between broken SoC and others. But I may be
> too optimistic...
If you agree with me that the second problem (interrupt-disable latency)
cannot be solved without resorting to heuristics (e.g., adding
read-backs or delays) then I think that should be the deciding point.
I'm responding to this message with an RFC of how the work-around could
be implemented (adding DT-support to the driver in the process). It
applies after having reverted the current workaround.
Note that only using the shadow interrupt mask when it is actually
needed does not add much extra code at all.
Thanks,
Johan
--
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/
Re: [PATCH] rtc: rtc-at91rm9200: use a variable for storing IMR
On Thu, Mar 28, 2013 at 02:20:17PM -0400, Douglas Gilbert wrote:
> On 13-03-28 05:57 AM, Johan Hovold wrote:
> > On Tue, Mar 26, 2013 at 05:09:59PM -0400, Douglas Gilbert wrote:
> >> On 13-03-26 03:27 PM, Johan Hovold wrote:
> >>> On Fri, Mar 15, 2013 at 06:37:12PM +0100, Nicolas Ferre wrote:
> >>>> On some revisions of AT91 SoCs, the RTC IMR register is not working.
> >>>> Instead of elaborating a workaround for that specific SoC or IP version,
> >>>> we simply use a software variable to store the Interrupt Mask Register
> >>>> and
> >>>> modify it for each enabling/disabling of an interrupt. The overhead of
> >>>> this
> >>>> is negligible anyway.
> >>>
> >>> The patch does not add any memory barriers or register read-backs when
> >>> manipulating the interrupt-mask variable. This could possibly lead to
> >>> spurious interrupts both when enabling and disabling the various
> >>> RTC-interrupts due to write reordering and bus latencies.
> >>>
> >>> Has this been considered? And is this reason enough for a more targeted
> >>> work-around so that the SOCs with functional RTC_IMR are not affected?
> >>
> >> The SoCs in question use a single embedded ARM926EJ-S and
> >> according to the Atmel documentation, that CPU's instruction
> >> set contains no barrier (or related) instructions.
> >
> > The ARM926EJ-S actually does have a Drain Write Buffer instruction but
> > it's not used by the ARM barrier-implementation unless
> > CONFIG_ARM_DMA_MEM_BUFFERABLE or CONFIG_SMP is set.
>
> The ARM926EJ-S is ARMv5 so CONFIG_ARM_DMA_MEM_BUFFERABLE is not
> available. SMP is not an option for arm/mach-at91.
Never said it was. I merely disputed the claim that the ARM926EJ-S has
no barrier instruction.
> > However, wmb() always implies a compiler barrier which is what is needed
> > in this case.
>
> Even if wmb() did anything, would it make this case "safe"?
As I write above, wmb() always implies a compiler barrier, which is is
sufficient to prevent write reordering on this SoC. In particular, wmb()
is defined as a compiler barrier on AT91.
> >> In the arch/arm/mach-at91 sub-tree of the kernel source
> >> I can find no use of the wmb() call. Also checked all drivers
> >> in the kernel containing "at91" and none called wmb().
> >
> > I/O-operations are normally not reordered, but this patch is faking a
> > hardware register and thus extra care needs to be taken.
> >
> > To repeat:
> >
> >> @@ -198,9 +203,12 @@ static int at91_rtc_alarm_irq_enable(struct device
> >> *dev, unsigned int enabled)
> >>
> >>if (enabled) {
> >>at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
> >> + at91_rtc_imr |= AT91_RTC_ALARM;
> >
> > Here a barrier is needed to prevent the compiler from reordering the two
> > writes (i.e., mask update and interrupt enable).
>
> Isn't either order potentially unsafe? So even if the compiler
> did foolishly re-order them, the sequence is still unsafe when
> a SYS interrupt splits those two lines (since the SYS interrupt
> is shared, it can occur at any time).
Good point. This would not cause any problem as the interrupt mask is
ANDed with the (hardware) status register in the interrupt handler, but
only if the status register is _guaranteed_ to be cleared before
updating the mask and that would require another wmb() after writing
SCCR above.
To avoid having to worry about such subtleties, a spinlock (and the
barriers it implies) seems like the most reasonable way to prevent the
race in this case. Note however that this only fixes the reordering part
of the problem. The register read back would still be needed below.
> >>at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
> >> - } else
> >> + } else {
> >>at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
> >
> > Here a barrier is again needed to prevent the compiler from reordering,
> > but we also need a register read back (of some RTC-register) before
> > updating the mask. Without the register read back, there could be a
> > window where the mask does not match the hardware state due to bus
> > latencies.
> >
> > Note that even with a register read back there is a (theoretical)
> > possibility that the interrupts have not yet been disabled when the fake
> > mask is updated. The only way to know for sure is to poll RTC_IMR but
> > that is the very register you're trying to emulate.
>
[RFC 5/5] rtc-at91rm9200: add support for at91sam9x5
Add support for the at91sam9x5-family which must use the shadow
interrupt mask due to a hardware issue.
---
drivers/rtc/rtc-at91rm9200.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 2921866..f3e351f 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -318,12 +318,20 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
*dev_id)
static const struct at91_rtc_config at91rm9200_config = {
};
+static const struct at91_rtc_config at91sam9x5_config = {
+ .use_shadow_imr = true,
+};
+
#if defined(CONFIG_OF)
static const struct of_device_id at91_rtc_dt_ids[] = {
{
.compatible = "atmel,at91rm9200-rtc",
.data = &at91rm9200_config,
},
+ {
+ .compatible = "atmel,at91sam9x5-rtc",
+ .data = &at91sam9x5_config,
+ },
/* terminator */
}
};
--
1.8.1.5
--
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/
[RFC 1/5] rtc-at91rm9200: add configuration support
Add configuration support which can be used to implement SoC-specific
workarounds for broken hardware.
---
drivers/rtc/rtc-at91rm9200.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 434ebc3..5bae0a1 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -40,6 +40,10 @@
#define AT91_RTC_EPOCH 1900UL /* just like arch/arm/common/rtctime.c
*/
+struct at91_rtc_config {
+};
+
+static const struct at91_rtc_config *at91_rtc_config;
static DECLARE_COMPLETION(at91_rtc_updated);
static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
@@ -248,6 +252,15 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
*dev_id)
return IRQ_NONE;/* not handled */
}
+static const struct at91_rtc_config at91rm9200_config = {
+};
+
+static const struct at91_rtc_config *
+at91_rtc_get_config(struct platform_device *pdev)
+{
+ return &at91rm9200_config;
+}
+
static const struct rtc_class_ops at91_rtc_ops = {
.read_time = at91_rtc_readtime,
.set_time = at91_rtc_settime,
@@ -266,6 +279,10 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
struct resource *regs;
int ret = 0;
+ at91_rtc_config = at91_rtc_get_config(pdev);
+ if (!at91_rtc_config)
+ return -ENODEV;
+
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
dev_err(&pdev->dev, "no mmio resource defined\n");
--
1.8.1.5
--
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/
[RFC 3/5] rtc-at91rm9200: refactor interrupt-register handling
Add accessors for the interrupt register.
This will allow us to easily add a shadow interrupt-mask register to
use on SoCs where the interrupt-mask register cannot be used.
---
drivers/rtc/rtc-at91rm9200.c | 43 +--
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 67260f9..cb4462d 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -50,6 +50,21 @@ static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
static int irq;
+static void at91_rtc_write_ier(u32 mask)
+{
+ at91_rtc_write(AT91_RTC_IER, mask);
+}
+
+static void at91_rtc_write_idr(u32 mask)
+{
+ at91_rtc_write(AT91_RTC_IDR, mask);
+}
+
+static u32 at91_rtc_read_imr(void)
+{
+ return at91_rtc_read(AT91_RTC_IMR);
+}
+
/*
* Decode time/date into rtc_time structure
*/
@@ -113,9 +128,9 @@ static int at91_rtc_settime(struct device *dev, struct
rtc_time *tm)
cr = at91_rtc_read(AT91_RTC_CR);
at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | AT91_RTC_UPDTIM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ACKUPD);
+ at91_rtc_write_ier(AT91_RTC_ACKUPD);
wait_for_completion(&at91_rtc_updated); /* wait for ACKUPD interrupt */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD);
+ at91_rtc_write_idr(AT91_RTC_ACKUPD);
at91_rtc_write(AT91_RTC_TIMR,
bin2bcd(tm->tm_sec) << 0
@@ -147,7 +162,7 @@ static int at91_rtc_readalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
tm->tm_year = at91_alarm_year - 1900;
- alrm->enabled = (at91_rtc_read(AT91_RTC_IMR) & AT91_RTC_ALARM)
+ alrm->enabled = (at91_rtc_read_imr() & AT91_RTC_ALARM)
? 1 : 0;
dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -172,7 +187,7 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
tm.tm_min = alrm->time.tm_min;
tm.tm_sec = alrm->time.tm_sec;
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+ at91_rtc_write_idr(AT91_RTC_ALARM);
at91_rtc_write(AT91_RTC_TIMALR,
bin2bcd(tm.tm_sec) << 0
| bin2bcd(tm.tm_min) << 8
@@ -185,7 +200,7 @@ static int at91_rtc_setalarm(struct device *dev, struct
rtc_wkalrm *alrm)
if (alrm->enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+ at91_rtc_write_ier(AT91_RTC_ALARM);
}
dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -201,9 +216,9 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
if (enabled) {
at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
- at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+ at91_rtc_write_ier(AT91_RTC_ALARM);
} else
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+ at91_rtc_write_idr(AT91_RTC_ALARM);
return 0;
}
@@ -212,7 +227,7 @@ static int at91_rtc_alarm_irq_enable(struct device *dev,
unsigned int enabled)
*/
static int at91_rtc_proc(struct device *dev, struct seq_file *seq)
{
- unsigned long imr = at91_rtc_read(AT91_RTC_IMR);
+ unsigned long imr = at91_rtc_read_imr();
seq_printf(seq, "update_IRQ\t: %s\n",
(imr & AT91_RTC_ACKUPD) ? "yes" : "no");
@@ -232,7 +247,7 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
unsigned int rtsr;
unsigned long events = 0;
- rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read(AT91_RTC_IMR);
+ rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read_imr();
if (rtsr) { /* this interrupt is shared! Is it ours? */
if (rtsr & AT91_RTC_ALARM)
events |= (RTC_AF | RTC_IRQF);
@@ -327,7 +342,7 @@ static int __init at91_rtc_probe(struct platform_device
*pdev)
at91_rtc_write(AT91_RTC_MR, 0); /* 24 hour mode */
/* Disable all interrupts */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+ at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
AT91_RTC_SECEV | AT91_RTC_TIMEV |
AT91_RTC_CALEV);
@@ -365,7 +380,7 @@ static int __exit at91_rtc_remove(struct platform_device
*pdev)
struct rtc_device *rtc = platform_get_drvdata(pdev);
/* Disable all interrupts */
- at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+ at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
AT91_RTC_SECEV | AT91_RTC_TIMEV |
AT91_RTC_CALEV);
free_irq(irq,
[RFC 4/5] rtc-at91rm9200: add shadow interrupt mask
Add shadow interrupt-mask register which can be used on SoCs where the
actual hardware register is broken.
Note that some care needs to be taken to make sure the shadow mask
corresponds to the actual hardware state. The added overhead is not an
issue for the non-broken SoCs due to the relatively infrequent
interrupt-mask updates. We do, however, only use the shadow mask value
as a fall-back when it actually needed as there is still a theoretical
possibility that the mask is incorrect (see the code for details).
---
drivers/rtc/rtc-at91rm9200.c | 49 +++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index cb4462d..2921866 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -42,6 +42,7 @@
#define AT91_RTC_EPOCH 1900UL /* just like arch/arm/common/rtctime.c
*/
struct at91_rtc_config {
+ bool use_shadow_imr;
};
static const struct at91_rtc_config *at91_rtc_config;
@@ -49,20 +50,66 @@ static DECLARE_COMPLETION(at91_rtc_updated);
static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
static void __iomem *at91_rtc_regs;
static int irq;
+static DEFINE_SPINLOCK(at91_rtc_lock);
+static u32 at91_rtc_shadow_imr;
static void at91_rtc_write_ier(u32 mask)
{
+ unsigned long flags;
+
+ /*
+* Lock needed to make sure shadow mask is updated before interrupts
+* are enabled.
+*/
+ spin_lock_irqsave(&at91_rtc_lock, flags);
+ at91_rtc_shadow_imr |= mask;
at91_rtc_write(AT91_RTC_IER, mask);
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
}
static void at91_rtc_write_idr(u32 mask)
{
+ unsigned long flags;
+
+ /*
+* Lock needed to make sure shadow mask is not updated before
+* interrupts have been disabled.
+*/
+ spin_lock_irqsave(&at91_rtc_lock, flags);
at91_rtc_write(AT91_RTC_IDR, mask);
+ /*
+* Register read back (of any RTC-register) needed to make sure
+* IDR-register write has reached the peripheral before updating
+* shadow mask.
+*
+* Note that there is still a possibility that the mask is updated
+* before interrupts have actually been disabled in hardware. The only
+* way to be certain would be to poll the IMR-register, which is is
+* the very register we are trying to emulate. The register read back
+* is a reasonable heuristic.
+*/
+ at91_rtc_read(AT91_RTC_SR);
+ at91_rtc_shadow_imr &= ~mask;
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
}
static u32 at91_rtc_read_imr(void)
{
- return at91_rtc_read(AT91_RTC_IMR);
+ unsigned long flags;
+ u32 mask;
+
+ if (at91_rtc_config->use_shadow_imr) {
+ /*
+* Lock not strictly necessary on UP.
+*/
+ spin_lock_irqsave(&at91_rtc_lock, flags);
+ mask = at91_rtc_shadow_imr;
+ spin_unlock_irqrestore(&at91_rtc_lock, flags);
+ } else {
+ mask = at91_rtc_read(AT91_RTC_IMR);
+ }
+
+ return mask;
}
/*
--
1.8.1.5
--
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/
[RFC 2/5] rtc-at91rm9200: add device-tree support
Add device tree support.
---
drivers/rtc/rtc-at91rm9200.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 5bae0a1..67260f9 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
@@ -255,9 +256,30 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
*dev_id)
static const struct at91_rtc_config at91rm9200_config = {
};
+#if defined(CONFIG_OF)
+static const struct of_device_id at91_rtc_dt_ids[] = {
+ {
+ .compatible = "atmel,at91rm9200-rtc",
+ .data = &at91rm9200_config,
+ },
+ /* terminator */
+ }
+};
+MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
+#endif
+
static const struct at91_rtc_config *
at91_rtc_get_config(struct platform_device *pdev)
{
+ const struct of_device_id *match;
+
+ if (pdev->dev.of_node) {
+ match = of_match_node(at91_rtc_dt_ids, pdev->dev.of_node);
+ if (!match)
+ return NULL;
+ return (const struct at91_rtc_config *)match->data;
+ }
+
return &at91rm9200_config;
}
@@ -403,6 +425,7 @@ static struct platform_driver at91_rtc_driver = {
.driver = {
.name = "at91_rtc",
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(at91_rtc_dt_ids),
.pm = at91_rtc_pm_ptr,
},
};
--
1.8.1.5
--
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/
Re: [RFC 2/5] rtc-at91rm9200: add device-tree support
On Fri, Mar 29, 2013 at 05:03:46PM +0100, Johan Hovold wrote:
> Add device tree support.
> ---
> drivers/rtc/rtc-at91rm9200.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
> index 5bae0a1..67260f9 100644
> --- a/drivers/rtc/rtc-at91rm9200.c
> +++ b/drivers/rtc/rtc-at91rm9200.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -255,9 +256,30 @@ static irqreturn_t at91_rtc_interrupt(int irq, void
> *dev_id)
> static const struct at91_rtc_config at91rm9200_config = {
> };
>
> +#if defined(CONFIG_OF)
> +static const struct of_device_id at91_rtc_dt_ids[] = {
> + {
> + .compatible = "atmel,at91rm9200-rtc",
> + .data = &at91rm9200_config,
>+ },
There's a missing brace here. Will fix after any further feedback.
> + /* terminator */
> + }
> +};
> +MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
> +#endif
--
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/
Re: [Fwd: [PATCH v2 0/4] TTY: port hangup and close fixes]
Hi Jiri,
On Tue, Mar 05, 2013 at 05:02:44PM +0100, Jiri Slaby wrote:
> On 02/28/2013 09:57 PM, Peter Hurley wrote:
> > Hi Jiri,
> >
> > Just wanted to make sure you saw this series.
>
> Hi, thanks for letting me know. Johan, care to CC Alan Cox and me (or at
> least LKML) when you're changing the TTY core next time?
Sorry about that. Thought it was a bit odd I didn't hear anything from
you actually. ;) Everything was posted to linux-serial (and Alan
initially), but I'll remember to CC you in the future as well.
> I have a couple of questions for 2/4:
>
> > Move HUPCL handling to port shutdown so that DTR is dropped also on
> > hang up (tty_port_close is a noop for hung-up ports).
>
> It makes sense, but I'm not sure -- is this expected, i.e. does this
> conform to standards and/or BSDs?
As Peter also mentioned, this is how serial_core (and another seven tty
drivers) work today.
There are currently seven drivers (counting usb-serial as one) that
manipulate DTR at open/close but do not drop DTR on hangup, and five of
those (including usb-serial) don't do it because they use the tty_port
implementation.
> > @@ -196,13 +196,20 @@ void tty_port_tty_set(struct tty_port *port, struct
> > tty_struct *tty)
> > }
> > EXPORT_SYMBOL(tty_port_tty_set);
>
> -static void tty_port_shutdown(struct tty_port *port)
> +static void tty_port_shutdown(struct tty_port *port, struct tty_struct
> *tty)
> {
> mutex_lock(&port->mutex);
> if (port->console)
> goto out;
>
> if (test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags)) {
> + /*
> +* Drop DTR/RTS if HUPCL is set. This causes any attached
> +* modem to hang up the line.
> +*/
> + if (!tty || tty->termios.c_cflag & HUPCL)
> > + tty_port_lower_dtr_rts(port);
> > +
>
> So you drop the line even thought the user didn't necessarily want to,
> in case the tty is gone already?
You have a point in that it might be better to do it the other way round
and not touch DTR unless we know for sure it was requested. [ But see my
answer to you next question as well. ]
Several drivers (including serial_core) had a similar construct in their
shutdown() but tty is never NULL when called from hangup in those cases.
> > @@ -225,15 +232,13 @@ void tty_port_hangup(struct tty_port *port)
> spin_lock_irqsave(&port->lock, flags);
> port->count = 0;
> port->flags &= ~ASYNC_NORMAL_ACTIVE;
> - if (port->tty) {
> + if (port->tty)
> set_bit(TTY_IO_ERROR, &port->tty->flags);
> - tty_kref_put(port->tty);
> - }
> - port->tty = NULL;
> spin_unlock_irqrestore(&port->lock, flags);
> > + tty_port_shutdown(port, port->tty);
>
> What prevents port->tty to be NULL here already?
Nothing, I'll get a new reference within the port lock section as you
just suggested elsewhere in this thread.
But this should never be the case when using both tty_port_close and
tty_port_hangup, as then port->tty will only be NULL if the port has
already been shut down, right?
> > + tty_port_tty_set(port, NULL);
> > wake_up_interruptible(&port->open_wait);
> > wake_up_interruptible(&port->delta_msr_wait);
> > - tty_port_shutdown(port);
>
> Did you investigate if the order matters here? I don't know, just curious...
Yes, I did. First, the order should not matter for blocked opens as they
will exit their wait loops based on tty_hung_up_p(filp) either way.
As for delta_msr_wait the changed order is actually preferred as it
allows the waiting process to return based on ASYNC_INITIALIZED. This is
also the order used by serial_core. Note however that the current
serial_core TIOCMIWAIT is broken in that it doesn't return on hangups at
all.
Perhaps I should separate this to a patch of its own, and send a fix
for serial_core TIOCMIWAIT as well.
Thanks,
Johan
> > @@ -452,11 +457,6 @@ int tty_port_close_start(struct tty_port *port,
> /* Flush the ldisc buffering */
> tty_ldisc_flush(tty);
>
> - /* Drop DTR/RTS if HUPCL is set. This causes any attached modem to
> - hang up the line */
> - if (tty->termios.c_cflag & HUPCL)
> - tty_port_lower_dtr_rts(port);
> -
> /* Don't call port->drop for the last reference. Callers will want
>to drop the last active reference in ->shutdown() or the tty
> >shutdown path */
--
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/
Re: [Fwd: [PATCH v2 0/4] TTY: port hangup and close fixes]
On Wed, Mar 06, 2013 at 02:14:56PM -0500, Peter Hurley wrote:
> On Wed, 2013-03-06 at 17:52 +0100, Johan Hovold wrote:
>
> > > > @@ -225,15 +232,13 @@ void tty_port_hangup(struct tty_port *port)
> > > spin_lock_irqsave(&port->lock, flags);
> > > port->count = 0;
> > > port->flags &= ~ASYNC_NORMAL_ACTIVE;
> > > - if (port->tty) {
> > > + if (port->tty)
> > > set_bit(TTY_IO_ERROR, &port->tty->flags);
> > > - tty_kref_put(port->tty);
> > > - }
> > > - port->tty = NULL;
> > > spin_unlock_irqrestore(&port->lock, flags);
> > > > + tty_port_shutdown(port, port->tty);
> > >
> > > What prevents port->tty to be NULL here already?
> >
> > Nothing, I'll get a new reference within the port lock section as you
> > just suggested elsewhere in this thread.
>
> Don't do that. Steal the tty and put the kref after like this:
Allright.
> > Yes, I did. First, the order should not matter for blocked opens as they
> > will exit their wait loops based on tty_hung_up_p(filp) either way.
>
> Only if the open() was ever successful, otherwise the filp won't be in
> the tty->tty_files list. That's why the blocking opens also check
> ASYNC_INITIALIZED (or ASYNCB_INITIALIZED depending on which they use).
> Which is why I said it was actually better to shutdown() first, then
> wake up the blocked opens.
ASYNC_INITIALIZED have also been cleared when the blocked opens are
being woken up from tty_port_close_end.
And the filp is added to tty_files before open() is called:
===>tty_add_file(tty, filp);
check_tty_count(tty, __func__);
if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
tty->driver->subtype == PTY_TYPE_MASTER)
noctty = 1;
#ifdef TTY_DEBUG_HANGUP
printk(KERN_DEBUG "%s: opening %s...\n", __func__, tty->name);
#endif
if (tty->ops->open)
===>retval = tty->ops->open(tty, filp);
so a blocked open must have hung_up_tty_fops when woken up from hangup,
right?
Either way, postponing wake-up somewhat in tty_port_hangup should be
fine.
Thanks,
Johan
--
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/
[PATCH v3 6/6] TTY: fix close of uninitialised ports
Make sure we do not make tty-driver callbacks or wait for port to drain
on uninitialised ports (e.g. when open failed) in
tty_port_close_start().
No callback, such as flush_buffer or wait_until_sent, needs to be made
on a port that has never been opened. Neither does it make much sense to
add drain delay for an uninitialised port.
Currently a drain delay of up to two seconds could be added when a tty
fails to open.
Signed-off-by: Johan Hovold
---
drivers/tty/tty_port.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 048cc85..d028df3 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -454,14 +454,16 @@ int tty_port_close_start(struct tty_port *port,
set_bit(ASYNCB_CLOSING, &port->flags);
tty->closing = 1;
spin_unlock_irqrestore(&port->lock, flags);
- /* Don't block on a stalled port, just pull the chain */
- if (tty->flow_stopped)
- tty_driver_flush_buffer(tty);
- if (test_bit(ASYNCB_INITIALIZED, &port->flags) &&
- port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent_from_close(tty, port->closing_wait);
- if (port->drain_delay)
- tty_port_drain_delay(port, tty);
+
+ if (test_bit(ASYNCB_INITIALIZED, &port->flags)) {
+ /* Don't block on a stalled port, just pull the chain */
+ if (tty->flow_stopped)
+ tty_driver_flush_buffer(tty);
+ if (port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
+ tty_wait_until_sent_from_close(tty, port->closing_wait);
+ if (port->drain_delay)
+ tty_port_drain_delay(port, tty);
+ }
/* Flush the ldisc buffering */
tty_ldisc_flush(tty);
--
1.8.1.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/
[PATCH v3 2/6] TTY: wake up processes last at hangup
Move wake up of processes on blocked-open and modem-status wait queues to after port shutdown at hangup. This way the woken up processes can use the ASYNC_INITIALIZED flag to detect port shutdown. Note that this is the order currently used by serial-core. Signed-off-by: Johan Hovold --- drivers/tty/tty_port.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index 57a061e..3de5918 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -231,9 +231,9 @@ void tty_port_hangup(struct tty_port *port) } port->tty = NULL; spin_unlock_irqrestore(&port->lock, flags); + tty_port_shutdown(port); wake_up_interruptible(&port->open_wait); wake_up_interruptible(&port->delta_msr_wait); - tty_port_shutdown(port); } EXPORT_SYMBOL(tty_port_hangup); -- 1.8.1.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/
