(Observed while running on Kontron nano-TT module.)
Certain BIOS/boot firmware may disable the tunnelcreek watchdog. Perhaps
because the mainboard or module provides it's own watchdog mechanism. In these
cases, lpc_sch should just go on and still provide the rest of the cells
it declares (GPIO and SMBus). The same logic could be applied to the other
resources, but I'll leave it to others to decide that.
This change makes the probe function tolerate the I/O range being disabled,
but not if the base address inside the register is not configured.
Cc: Alexander Stein <[email protected]>
Cc: Denis Turischev <[email protected]>
Signed-off-by: Jean-François Dagenais <[email protected]>
---
drivers/mfd/lpc_sch.c | 39 ++++++++++++++++++++-------------------
1 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/mfd/lpc_sch.c b/drivers/mfd/lpc_sch.c
index ea1169b..6621cd3 100644
--- a/drivers/mfd/lpc_sch.c
+++ b/drivers/mfd/lpc_sch.c
@@ -127,26 +127,27 @@ static int __devinit lpc_sch_probe(struct pci_dev *dev,
if (id->device == PCI_DEVICE_ID_INTEL_ITC_LPC) {
pci_read_config_dword(dev, WDTBASE, &base_addr_cfg);
- if (!(base_addr_cfg & (1 << 31))) {
- dev_err(&dev->dev, "Decode of the WDT I/O range disabled\n");
- ret = -ENODEV;
- goto out_dev;
+ if (base_addr_cfg & (1 << 31)) {
+ base_addr = (unsigned short)base_addr_cfg;
+ if (base_addr == 0) {
+ dev_err(&dev->dev,
+ "I/O space for WDT uninitialized\n");
+ ret = -ENODEV;
+ goto out_dev;
+ }
+
+ wdt_sch_resource.start = base_addr;
+ wdt_sch_resource.end = base_addr + WDT_IO_SIZE - 1;
+
+ for (i = 0; i < ARRAY_SIZE(tunnelcreek_cells); i++)
+ tunnelcreek_cells[i].id = id->device;
+
+ ret = mfd_add_devices(&dev->dev, 0, tunnelcreek_cells,
+ ARRAY_SIZE(tunnelcreek_cells), NULL, 0);
+ } else {
+ dev_warn(&dev->dev, "Decode of the WDT I/O range disabled, tunnelcreek watchdog timer maybe disabled by firmware or BIOS");
+ return 0;
}
- base_addr = (unsigned short)base_addr_cfg;
- if (base_addr == 0) {
- dev_err(&dev->dev, "I/O space for WDT uninitialized\n");
- ret = -ENODEV;
- goto out_dev;
- }
-
- wdt_sch_resource.start = base_addr;
- wdt_sch_resource.end = base_addr + WDT_IO_SIZE - 1;
-
- for (i = 0; i < ARRAY_SIZE(tunnelcreek_cells); i++)
- tunnelcreek_cells[i].id = id->device;
-
- ret = mfd_add_devices(&dev->dev, 0, tunnelcreek_cells,
- ARRAY_SIZE(tunnelcreek_cells), NULL, 0);
}
return ret;
--
1.7.9
On Friday 11 May 2012 15:15:22, Jean-Fran?ois Dagenais wrote:
> (Observed while running on Kontron nano-TT module.)
>
> Certain BIOS/boot firmware may disable the tunnelcreek watchdog. Perhaps
> because the mainboard or module provides it's own watchdog mechanism. In these
> cases, lpc_sch should just go on and still provide the rest of the cells
> it declares (GPIO and SMBus). The same logic could be applied to the other
> resources, but I'll leave it to others to decide that.
>
> This change makes the probe function tolerate the I/O range being disabled,
> but not if the base address inside the register is not configured.
>
> Cc: Alexander Stein <[email protected]>
> Cc: Denis Turischev <[email protected]>
> Signed-off-by: Jean-Fran?ois Dagenais <[email protected]>
> ---
> drivers/mfd/lpc_sch.c | 39 ++++++++++++++++++++-------------------
> 1 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mfd/lpc_sch.c b/drivers/mfd/lpc_sch.c
> index ea1169b..6621cd3 100644
> --- a/drivers/mfd/lpc_sch.c
> +++ b/drivers/mfd/lpc_sch.c
> @@ -127,26 +127,27 @@ static int __devinit lpc_sch_probe(struct pci_dev *dev,
>
> if (id->device == PCI_DEVICE_ID_INTEL_ITC_LPC) {
> pci_read_config_dword(dev, WDTBASE, &base_addr_cfg);
> - if (!(base_addr_cfg & (1 << 31))) {
> - dev_err(&dev->dev, "Decode of the WDT I/O range disabled\n");
> - ret = -ENODEV;
> - goto out_dev;
> + if (base_addr_cfg & (1 << 31)) {
> + base_addr = (unsigned short)base_addr_cfg;
> + if (base_addr == 0) {
> + dev_err(&dev->dev,
> + "I/O space for WDT uninitialized\n");
> + ret = -ENODEV;
> + goto out_dev;
> + }
> +
> + wdt_sch_resource.start = base_addr;
> + wdt_sch_resource.end = base_addr + WDT_IO_SIZE - 1;
> +
> + for (i = 0; i < ARRAY_SIZE(tunnelcreek_cells); i++)
> + tunnelcreek_cells[i].id = id->device;
> +
> + ret = mfd_add_devices(&dev->dev, 0, tunnelcreek_cells,
> + ARRAY_SIZE(tunnelcreek_cells), NULL, 0);
> + } else {
> + dev_warn(&dev->dev, "Decode of the WDT I/O range disabled, tunnelcreek watchdog timer maybe disabled by firmware or BIOS");
> + return 0;
Are you sure this is return is correct? I think as long as the watchdog is the last device in this probe it should be fine.
But not if any other device is added below later.
> }
> - base_addr = (unsigned short)base_addr_cfg;
> - if (base_addr == 0) {
> - dev_err(&dev->dev, "I/O space for WDT uninitialized\n");
> - ret = -ENODEV;
> - goto out_dev;
> - }
> -
> - wdt_sch_resource.start = base_addr;
> - wdt_sch_resource.end = base_addr + WDT_IO_SIZE - 1;
> -
> - for (i = 0; i < ARRAY_SIZE(tunnelcreek_cells); i++)
> - tunnelcreek_cells[i].id = id->device;
> -
> - ret = mfd_add_devices(&dev->dev, 0, tunnelcreek_cells,
> - ARRAY_SIZE(tunnelcreek_cells), NULL, 0);
> }
>
> return ret;
>
Despite that:
Acked-By: Alexander Stein <[email protected]>
Regards,
Alexander
--
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
August-Bebel-Str. 29
D-07973 Greiz
Tel: +49-3661-6279-0, Fax: +49-3661-6279-99
eMail: [email protected]
Internet: http://www.systec-electronic.com
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Jena, HRB 205563
On May 14, 2012, at 2:37, Alexander Stein wrote:
> On Friday 11 May 2012 15:15:22, Jean-Fran?ois Dagenais wrote:
>> (Observed while running on Kontron nano-TT module.)
>>
>> Certain BIOS/boot firmware may disable the tunnelcreek watchdog. Perhaps
>> because the mainboard or module provides it's own watchdog mechanism. In these
>> cases, lpc_sch should just go on and still provide the rest of the cells
>> it declares (GPIO and SMBus). The same logic could be applied to the other
>> resources, but I'll leave it to others to decide that.
>>
>> This change makes the probe function tolerate the I/O range being disabled,
>> but not if the base address inside the register is not configured.
>>
>> Cc: Alexander Stein <[email protected]>
>> Cc: Denis Turischev <[email protected]>
>> Signed-off-by: Jean-Fran?ois Dagenais <[email protected]>
>> ---
>> drivers/mfd/lpc_sch.c | 39 ++++++++++++++++++++-------------------
>> 1 files changed, 20 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/mfd/lpc_sch.c b/drivers/mfd/lpc_sch.c
>> index ea1169b..6621cd3 100644
>> --- a/drivers/mfd/lpc_sch.c
>> +++ b/drivers/mfd/lpc_sch.c
>> @@ -127,26 +127,27 @@ static int __devinit lpc_sch_probe(struct pci_dev *dev,
>>
>> if (id->device == PCI_DEVICE_ID_INTEL_ITC_LPC) {
>> pci_read_config_dword(dev, WDTBASE, &base_addr_cfg);
>> - if (!(base_addr_cfg & (1 << 31))) {
>> - dev_err(&dev->dev, "Decode of the WDT I/O range disabled\n");
>> - ret = -ENODEV;
>> - goto out_dev;
>> + if (base_addr_cfg & (1 << 31)) {
>> + base_addr = (unsigned short)base_addr_cfg;
>> + if (base_addr == 0) {
>> + dev_err(&dev->dev,
>> + "I/O space for WDT uninitialized\n");
>> + ret = -ENODEV;
>> + goto out_dev;
>> + }
>> +
>> + wdt_sch_resource.start = base_addr;
>> + wdt_sch_resource.end = base_addr + WDT_IO_SIZE - 1;
>> +
>> + for (i = 0; i < ARRAY_SIZE(tunnelcreek_cells); i++)
>> + tunnelcreek_cells[i].id = id->device;
>> +
>> + ret = mfd_add_devices(&dev->dev, 0, tunnelcreek_cells,
>> + ARRAY_SIZE(tunnelcreek_cells), NULL, 0);
>> + } else {
>> + dev_warn(&dev->dev, "Decode of the WDT I/O range disabled, tunnelcreek watchdog timer maybe disabled by firmware or BIOS");
>> + return 0;
>
> Are you sure this is return is correct? I think as long as the watchdog is the last device in this probe it should be fine.
> But not if any other device is added below later.
It is the last one, and yes, the next developers will have to keep it that way, or change the behaviour accordingly.
>
>> }
>> - base_addr = (unsigned short)base_addr_cfg;
>> - if (base_addr == 0) {
>> - dev_err(&dev->dev, "I/O space for WDT uninitialized\n");
>> - ret = -ENODEV;
>> - goto out_dev;
>> - }
>> -
>> - wdt_sch_resource.start = base_addr;
>> - wdt_sch_resource.end = base_addr + WDT_IO_SIZE - 1;
>> -
>> - for (i = 0; i < ARRAY_SIZE(tunnelcreek_cells); i++)
>> - tunnelcreek_cells[i].id = id->device;
>> -
>> - ret = mfd_add_devices(&dev->dev, 0, tunnelcreek_cells,
>> - ARRAY_SIZE(tunnelcreek_cells), NULL, 0);
>> }
>>
>> return ret;
>>
>
> Despite that:
> Acked-By: Alexander Stein <[email protected]>
Thanks.
>
> Regards,
> Alexander
> --
> Dipl.-Inf. Alexander Stein
>
> SYS TEC electronic GmbH
> August-Bebel-Str. 29
> D-07973 Greiz
>
> Tel: +49-3661-6279-0, Fax: +49-3661-6279-99
> eMail: [email protected]
> Internet: http://www.systec-electronic.com
>
> Managing Director: Dipl.-Phys. Siegmar Schmidt
> Commercial registry: Amtsgericht Jena, HRB 205563