Skip to contentSkip to conversation

Introduce MEN 14F021P00 Board Management Controller driver set

PATCH Superseded Andreas Werner 3 participants 12 messages May 16, 2014 – May 20, 2014

Subsystems (4)

v1 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Andreas Werner

author maintainer
#0 lore raw

cover Introduce MEN 14F021P00 Board Management Controller driver set

This patch set add support for the MEN 14F021P00 Board Management Controller called BMC.

The BMC is a PIC Mikrocontroller which assembled on almost all
of our 3U Compact PCI CPU boards and a few Box PCs.
The main part of the BMC is to start and monitor the board,
but there are a lot more features which can be accessed using an I2C Host interface.

Features supported in this Patchset:
- Watchdog
- LEDs

The Patchset includes a MFD Core driver, Watchdog and LEDs driver.

Andreas Werner (3):
drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver drivers/watchdog/menf21bmc_wd: introduce MEN 14F021P00 BMC Watchdog
driver
drivers/leds/leds-menf21bmc: introduce MEN 14F021P00 BMC LED driver

 drivers/leds/Kconfig            |   6 +
 drivers/leds/Makefile           |   2 +-
 drivers/leds/leds-menf21bmc.c   | 135 +++++++++++++++++++++
 drivers/mfd/Kconfig             |  12 ++
 drivers/mfd/Makefile            |   1 +
 drivers/mfd/menf21bmc.c         | 193 +++++++++++++++++++++++++++++
 drivers/watchdog/Kconfig        |   7 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/menf21bmc_wd.c | 263 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/menf21bmc.h   |  32 +++++
 10 files changed, 651 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-menf21bmc.c
 create mode 100644 drivers/mfd/menf21bmc.c
 create mode 100644 drivers/watchdog/menf21bmc_wd.c
 create mode 100644 include/linux/mfd/menf21bmc.h
Signature · 1 line
1.9.2
author maintainer
#1 lore raw

PATCH 1/3 drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver

The MEN 14F021P00 Board Management Controller provides an
I2C interface to the host to access the feature implemented in the BMC. The BMC is a PIC Microntroller assembled on CPCI Card from MEN Mikroelektronik and on a few Box/Display Computer.

Added MFD Core driver, supporting the I2C communication to the device.

The MFD driver currently supports the following features:
- Watchdog
- LEDs

---
 drivers/mfd/Kconfig           |  12 +++
 drivers/mfd/Makefile          |   1 +
 drivers/mfd/menf21bmc.c       | 193 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/menf21bmc.h |  32 +++++++
 4 files changed, 238 insertions(+)
 create mode 100644 drivers/mfd/menf21bmc.c
 create mode 100644 include/linux/mfd/menf21bmc.h
drivers/mfd/Kconfig +12 -0
index ab5a43c..7c2e8d2 100644
@@ -427,6 +427,18 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MENF21BMC
+	tristate "MEN 14F021P00 Board Management Controller Support"
+	depends on I2C=y
+	select MFD_CORE
+	help
+	  Say yes here to add support for the MEN 14F021P00 BMC
+	  which is a Board Management Controller connected to the I2C bus.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
+
 config EZX_PCAP
 	bool "Motorola EZXPCAP Support"
 	depends on SPI_MASTER
drivers/mfd/Makefile +1 -0
index 5913208..8f2be38 100644
@@ -167,3 +167,4 @@ obj-$(CONFIG_MFD_AS3711)	+= as3711.o
 obj-$(CONFIG_MFD_AS3722)	+= as3722.o
 obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
+obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
drivers/mfd/menf21bmc.c new file +193 -0

193 lines not on this page · raw message

include/linux/mfd/menf21bmc.h new file +32 -0
new file mode 100644
index 0000000..49df41fe
@@ -0,0 +1,32 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC)
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#ifndef MENF21BMC_H
+#define MENF21BMC_H
+
+struct menf21bmc {
+	struct i2c_client *client;
+	struct mutex lock;
+
+	int (*read_word_data)(struct i2c_client *client,
+						u8 reg, uint16_t *val);
+	int (*read_byte_data)(struct i2c_client *client, u8 reg, uint8_t *val);
+	int (*write_word_data)(struct i2c_client *client, u8 reg, u16 val);
+	int (*write_byte_data)(struct i2c_client *client, u8 reg, u8 val);
+	int (*write_byte)(struct i2c_client *client, u8 val);
+};
+
+#endif /* MENF21BMC_H */
+
+
Signature · 1 line
1.9.2
maintainer
#2 lore raw

Re: [PATCH 1/3] drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver

Quoted 105 lines · Andreas Werner

The MEN 14F021P00 Board Management Controller provides an
I2C interface to the host to access the feature implemented in the BMC. The BMC is a PIC Microntroller assembled on CPCI Card from MEN Mikroelektronik and on a few Box/Display Computer.

Added MFD Core driver, supporting the I2C communication to the device.

The MFD driver currently supports the following features:
- Watchdog
- LEDs

---
  drivers/mfd/Kconfig           |  12 +++
  drivers/mfd/Makefile          |   1 +
  drivers/mfd/menf21bmc.c       | 193 ++++++++++++++++++++++++++++++++++++++++++
  include/linux/mfd/menf21bmc.h |  32 +++++++
  4 files changed, 238 insertions(+)

create mode 100644 drivers/mfd/menf21bmc.c create mode 100644 include/linux/mfd/menf21bmc.h

drivers/mfd/Kconfig +12 -0
index ab5a43c..7c2e8d2 100644
@@ -427,6 +427,18 @@ config MFD_MAX8998
  	  additional drivers must be enabled in order to use the functionality
  	  of the device.
 
+config MFD_MENF21BMC
+	tristate "MEN 14F021P00 Board Management Controller Support"
+	depends on I2C=y
+	select MFD_CORE
+	help
+	  Say yes here to add support for the MEN 14F021P00 BMC
+	  which is a Board Management Controller connected to the I2C bus.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
+
  config EZX_PCAP
  	bool "Motorola EZXPCAP Support"
  	depends on SPI_MASTER
drivers/mfd/Makefile +1 -0
index 5913208..8f2be38 100644
@@ -167,3 +167,4 @@ obj-$(CONFIG_MFD_AS3711)	+= as3711.o
  obj-$(CONFIG_MFD_AS3722)	+= as3722.o
  obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
  obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
+obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
drivers/mfd/menf21bmc.c new file +46 -0
new file mode 100644
index 0000000..77de1a8
@@ -0,0 +1,193 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC) MFD Core Driver.
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/menf21bmc.h>
+
+#define BMC_CMD_REV_MAJOR	0x80
+#define BMC_CMD_REV_MINOR	0x81
+#define BMC_CMD_REV_MAIN	0x82
+#define BMC_CMD_REV_BUILD	0x83
+#define BMC_CMD_REV_VERI	0x84
+
+static struct mfd_cell menf21bmc_cell[] = {
+	{
+		.name = "menf21bmc_wd",
+	},
+	{
+		.name = "menf21bmc_led",
+	},
+};
+
+static int
+menf21bmc_read_byte_data(struct i2c_client *client, u8 reg, uint8_t *val)
+{
+	int ret;
+	struct menf21bmc *data = i2c_get_clientdata(client);
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_read_byte_data(client, reg);
+	mutex_unlock(&data->lock);
+

drivers/mfd/menf21bmc.c line 56

Quoted 10 lines
drivers/mfd/menf21bmc.c new file +10 -0
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read byte at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+
+	return 0;
+}
+

I personally would prefer if you would retain the original API, which returns both value and error code as return value. I don't see the benefit of changing the API as you did - it just complicates the code.

Thanks,
Guenter

author maintainer
#3 lore raw

Re: [PATCH 1/3] drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver

Quoted 120 lines · Guenter Roeck
Quoted 105 lines · Andreas Werner

The MEN 14F021P00 Board Management Controller provides an
I2C interface to the host to access the feature implemented in the BMC. The BMC is a PIC Microntroller assembled on CPCI Card from MEN Mikroelektronik and on a few Box/Display Computer.

Added MFD Core driver, supporting the I2C communication to the device.

The MFD driver currently supports the following features:
- Watchdog
- LEDs

---
 drivers/mfd/Kconfig           |  12 +++
 drivers/mfd/Makefile          |   1 +
 drivers/mfd/menf21bmc.c       | 193 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/menf21bmc.h |  32 +++++++
 4 files changed, 238 insertions(+)
 create mode 100644 drivers/mfd/menf21bmc.c
 create mode 100644 include/linux/mfd/menf21bmc.h
drivers/mfd/Kconfig +12 -0
index ab5a43c..7c2e8d2 100644
@@ -427,6 +427,18 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MENF21BMC
+	tristate "MEN 14F021P00 Board Management Controller Support"
+	depends on I2C=y
+	select MFD_CORE
+	help
+	  Say yes here to add support for the MEN 14F021P00 BMC
+	  which is a Board Management Controller connected to the I2C bus.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
+
 config EZX_PCAP
 	bool "Motorola EZXPCAP Support"
 	depends on SPI_MASTER
drivers/mfd/Makefile +1 -0
index 5913208..8f2be38 100644
@@ -167,3 +167,4 @@ obj-$(CONFIG_MFD_AS3711)	+= as3711.o
 obj-$(CONFIG_MFD_AS3722)	+= as3722.o
 obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
+obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
drivers/mfd/menf21bmc.c new file +46 -0
new file mode 100644
index 0000000..77de1a8
@@ -0,0 +1,193 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC) MFD Core Driver.
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/menf21bmc.h>
+
+#define BMC_CMD_REV_MAJOR	0x80
+#define BMC_CMD_REV_MINOR	0x81
+#define BMC_CMD_REV_MAIN	0x82
+#define BMC_CMD_REV_BUILD	0x83
+#define BMC_CMD_REV_VERI	0x84
+
+static struct mfd_cell menf21bmc_cell[] = {
+	{
+		.name = "menf21bmc_wd",
+	},
+	{
+		.name = "menf21bmc_led",
+	},
+};
+
+static int
+menf21bmc_read_byte_data(struct i2c_client *client, u8 reg, uint8_t *val)
+{
+	int ret;
+	struct menf21bmc *data = i2c_get_clientdata(client);
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_read_byte_data(client, reg);
+	mutex_unlock(&data->lock);
+

drivers/mfd/menf21bmc.c line 56

Quoted 10 lines
drivers/mfd/menf21bmc.c new file +10 -0
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read byte at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+
+	return 0;
+}
+

I personally would prefer if you would retain the original API, which returns both value and error code as return value. I don't see the benefit of changing the API as you did - it just complicates the code.

Quoted 3 lines

Thanks,
Guenter

I think it is ok for a wrapper like this because it does not really change an API because it is menf21bmc related.

I did the same in rtc-rx8581, and it was already done in rtc_ds1307.c.

I personally prefer this way because the return of the function and the return value of the i2c_smbus_read - which is the value read from the device - are seperated.

If it is a no go for the mfd/wdt core i can change it back to the original i2c_smbus API. If it is ok on your side for a wrapper like this, we can let it as it is.

Thanks
Andy

maintainer
#4 lore raw

Re: [PATCH 1/3] drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver

Quoted 127 lines · Andreas Werner
Quoted 120 lines · Guenter Roeck
Quoted 105 lines · Andreas Werner

The MEN 14F021P00 Board Management Controller provides an
I2C interface to the host to access the feature implemented in the BMC. The BMC is a PIC Microntroller assembled on CPCI Card from MEN Mikroelektronik and on a few Box/Display Computer.

Added MFD Core driver, supporting the I2C communication to the device.

The MFD driver currently supports the following features:
- Watchdog
- LEDs

---
  drivers/mfd/Kconfig           |  12 +++
  drivers/mfd/Makefile          |   1 +
  drivers/mfd/menf21bmc.c       | 193 ++++++++++++++++++++++++++++++++++++++++++
  include/linux/mfd/menf21bmc.h |  32 +++++++
  4 files changed, 238 insertions(+)

create mode 100644 drivers/mfd/menf21bmc.c create mode 100644 include/linux/mfd/menf21bmc.h

drivers/mfd/Kconfig +12 -0
index ab5a43c..7c2e8d2 100644
@@ -427,6 +427,18 @@ config MFD_MAX8998
  	  additional drivers must be enabled in order to use the functionality
  	  of the device.
 
+config MFD_MENF21BMC
+	tristate "MEN 14F021P00 Board Management Controller Support"
+	depends on I2C=y
+	select MFD_CORE
+	help
+	  Say yes here to add support for the MEN 14F021P00 BMC
+	  which is a Board Management Controller connected to the I2C bus.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
+
  config EZX_PCAP
  	bool "Motorola EZXPCAP Support"
  	depends on SPI_MASTER
drivers/mfd/Makefile +1 -0
index 5913208..8f2be38 100644
@@ -167,3 +167,4 @@ obj-$(CONFIG_MFD_AS3711)	+= as3711.o
  obj-$(CONFIG_MFD_AS3722)	+= as3722.o
  obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
  obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
+obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
drivers/mfd/menf21bmc.c new file +46 -0
new file mode 100644
index 0000000..77de1a8
@@ -0,0 +1,193 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC) MFD Core Driver.
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/menf21bmc.h>
+
+#define BMC_CMD_REV_MAJOR	0x80
+#define BMC_CMD_REV_MINOR	0x81
+#define BMC_CMD_REV_MAIN	0x82
+#define BMC_CMD_REV_BUILD	0x83
+#define BMC_CMD_REV_VERI	0x84
+
+static struct mfd_cell menf21bmc_cell[] = {
+	{
+		.name = "menf21bmc_wd",
+	},
+	{
+		.name = "menf21bmc_led",
+	},
+};
+
+static int
+menf21bmc_read_byte_data(struct i2c_client *client, u8 reg, uint8_t *val)
+{
+	int ret;
+	struct menf21bmc *data = i2c_get_clientdata(client);
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_read_byte_data(client, reg);
+	mutex_unlock(&data->lock);
+

drivers/mfd/menf21bmc.c line 56

Quoted 10 lines
drivers/mfd/menf21bmc.c new file +10 -0
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read byte at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+
+	return 0;
+}
+

I personally would prefer if you would retain the original API, which returns both value and error code as return value. I don't see the benefit of changing the API as you did - it just complicates the code.

Quoted 3 lines

Thanks,
Guenter

I think it is ok for a wrapper like this because it does not really change an API because it is menf21bmc related.

Quoted 8 lines

I did the same in rtc-rx8581, and it was already done in rtc_ds1307.c.

I personally prefer this way because the return of the function and the return value of the i2c_smbus_read - which is the value read from the device - are seperated.

If it is a no go for the mfd/wdt core i can change it back to the original i2c_smbus API. If it is ok on your side for a wrapper like this, we can let it as it is.

Not my call to make; depends on the mfd maintainer. I would not accept it, as I see it as unnecessary, adding complexity, and pointless. Sure, you can always find examples for everything somewhere in the kernel, but that doesn't make it better.

Guenter

#5 lore raw

Re: [PATCH 1/3] drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver

Quoted 133 lines · Guenter Roeck
Quoted 132 lines · Andreas Werner
Quoted 120 lines · Guenter Roeck
Quoted 105 lines · Andreas Werner

The MEN 14F021P00 Board Management Controller provides an
I2C interface to the host to access the feature implemented in the BMC. The BMC is a PIC Microntroller assembled on CPCI Card from MEN Mikroelektronik and on a few Box/Display Computer.

Added MFD Core driver, supporting the I2C communication to the device.

The MFD driver currently supports the following features:
- Watchdog
- LEDs

---
 drivers/mfd/Kconfig           |  12 +++
 drivers/mfd/Makefile          |   1 +
 drivers/mfd/menf21bmc.c       | 193 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/menf21bmc.h |  32 +++++++
 4 files changed, 238 insertions(+)
 create mode 100644 drivers/mfd/menf21bmc.c
 create mode 100644 include/linux/mfd/menf21bmc.h
drivers/mfd/Kconfig +12 -0
index ab5a43c..7c2e8d2 100644
@@ -427,6 +427,18 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MENF21BMC
+	tristate "MEN 14F021P00 Board Management Controller Support"
+	depends on I2C=y
+	select MFD_CORE
+	help
+	  Say yes here to add support for the MEN 14F021P00 BMC
+	  which is a Board Management Controller connected to the I2C bus.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
+
 config EZX_PCAP
 	bool "Motorola EZXPCAP Support"
 	depends on SPI_MASTER
drivers/mfd/Makefile +1 -0
index 5913208..8f2be38 100644
@@ -167,3 +167,4 @@ obj-$(CONFIG_MFD_AS3711)	+= as3711.o
 obj-$(CONFIG_MFD_AS3722)	+= as3722.o
 obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
+obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
drivers/mfd/menf21bmc.c new file +46 -0
new file mode 100644
index 0000000..77de1a8
@@ -0,0 +1,193 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC) MFD Core Driver.
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/menf21bmc.h>
+
+#define BMC_CMD_REV_MAJOR	0x80
+#define BMC_CMD_REV_MINOR	0x81
+#define BMC_CMD_REV_MAIN	0x82
+#define BMC_CMD_REV_BUILD	0x83
+#define BMC_CMD_REV_VERI	0x84
+
+static struct mfd_cell menf21bmc_cell[] = {
+	{
+		.name = "menf21bmc_wd",
+	},
+	{
+		.name = "menf21bmc_led",
+	},
+};
+
+static int
+menf21bmc_read_byte_data(struct i2c_client *client, u8 reg, uint8_t *val)
+{
+	int ret;
+	struct menf21bmc *data = i2c_get_clientdata(client);
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_read_byte_data(client, reg);
+	mutex_unlock(&data->lock);
+

drivers/mfd/menf21bmc.c line 56

Quoted 10 lines
drivers/mfd/menf21bmc.c new file +10 -0
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read byte at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+
+	return 0;
+}
+

I personally would prefer if you would retain the original API, which returns both value and error code as return value. I don't see the benefit of changing the API as you did - it just complicates the code.

Quoted 3 lines

Thanks,
Guenter

I think it is ok for a wrapper like this because it does not really change an API because it is menf21bmc related.

I did the same in rtc-rx8581, and it was already done in rtc_ds1307.c.

I personally prefer this way because the return of the function and the return value of the i2c_smbus_read - which is the value read from the device - are seperated.

Quoted 7 lines
Quoted 3 lines

If it is a no go for the mfd/wdt core i can change it back to the original i2c_smbus API. If it is ok on your side for a wrapper like this, we can let it as it is.

Not my call to make; depends on the mfd maintainer. I would not accept it, as I see it as unnecessary, adding complexity, and pointless. Sure, you can always find examples for everything somewhere in the kernel, but that doesn't make it better.

I'm inclined to agree with Guenter, this patch is just noise.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author maintainer
#6 lore raw

Re: [PATCH 1/3] drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver

Quoted 139 lines · Lee Jones
Quoted 137 lines · Guenter Roeck
Quoted 135 lines · Andreas Werner
Quoted 120 lines · Guenter Roeck
Quoted 105 lines · Andreas Werner

The MEN 14F021P00 Board Management Controller provides an
I2C interface to the host to access the feature implemented in the BMC. The BMC is a PIC Microntroller assembled on CPCI Card from MEN Mikroelektronik and on a few Box/Display Computer.

Added MFD Core driver, supporting the I2C communication to the device.

The MFD driver currently supports the following features:
- Watchdog
- LEDs

---
 drivers/mfd/Kconfig           |  12 +++
 drivers/mfd/Makefile          |   1 +
 drivers/mfd/menf21bmc.c       | 193 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/menf21bmc.h |  32 +++++++
 4 files changed, 238 insertions(+)
 create mode 100644 drivers/mfd/menf21bmc.c
 create mode 100644 include/linux/mfd/menf21bmc.h
drivers/mfd/Kconfig +12 -0
index ab5a43c..7c2e8d2 100644
@@ -427,6 +427,18 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MENF21BMC
+	tristate "MEN 14F021P00 Board Management Controller Support"
+	depends on I2C=y
+	select MFD_CORE
+	help
+	  Say yes here to add support for the MEN 14F021P00 BMC
+	  which is a Board Management Controller connected to the I2C bus.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
+
 config EZX_PCAP
 	bool "Motorola EZXPCAP Support"
 	depends on SPI_MASTER
drivers/mfd/Makefile +1 -0
index 5913208..8f2be38 100644
@@ -167,3 +167,4 @@ obj-$(CONFIG_MFD_AS3711)	+= as3711.o
 obj-$(CONFIG_MFD_AS3722)	+= as3722.o
 obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
+obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
drivers/mfd/menf21bmc.c new file +46 -0
new file mode 100644
index 0000000..77de1a8
@@ -0,0 +1,193 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC) MFD Core Driver.
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/menf21bmc.h>
+
+#define BMC_CMD_REV_MAJOR	0x80
+#define BMC_CMD_REV_MINOR	0x81
+#define BMC_CMD_REV_MAIN	0x82
+#define BMC_CMD_REV_BUILD	0x83
+#define BMC_CMD_REV_VERI	0x84
+
+static struct mfd_cell menf21bmc_cell[] = {
+	{
+		.name = "menf21bmc_wd",
+	},
+	{
+		.name = "menf21bmc_led",
+	},
+};
+
+static int
+menf21bmc_read_byte_data(struct i2c_client *client, u8 reg, uint8_t *val)
+{
+	int ret;
+	struct menf21bmc *data = i2c_get_clientdata(client);
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_read_byte_data(client, reg);
+	mutex_unlock(&data->lock);
+

drivers/mfd/menf21bmc.c line 56

Quoted 10 lines
drivers/mfd/menf21bmc.c new file +10 -0
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read byte at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+
+	return 0;
+}
+

I personally would prefer if you would retain the original API, which returns both value and error code as return value. I don't see the benefit of changing the API as you did - it just complicates the code.

Quoted 3 lines

Thanks,
Guenter

I think it is ok for a wrapper like this because it does not really change an API because it is menf21bmc related.

I did the same in rtc-rx8581, and it was already done in rtc_ds1307.c.

I personally prefer this way because the return of the function and the return value of the i2c_smbus_read - which is the value read from the device - are seperated.

If it is a no go for the mfd/wdt core i can change it back to the original i2c_smbus API. If it is ok on your side for a wrapper like this, we can let it as it is.

Quoted 5 lines
Quoted 3 lines

Not my call to make; depends on the mfd maintainer. I would not accept it, as I see it as unnecessary, adding complexity, and pointless. Sure, you can always find examples for everything somewhere in the kernel, but that doesn't make it better.

I'm inclined to agree with Guenter, this patch is just noise.

Ok no problem, i will fix it and resend the patch. For now I know it for the future :-)

Any other comments about the patch series?

Quoted 6 lines
Signature · 5 lines
-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Regards
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

Re: [PATCH 1/3] drivers/mfd/menf21bmc: introduce MEN 14F021P00 BMC MFD Core driver

Quoted 10 lines
Quoted 5 lines
Quoted 3 lines

Not my call to make; depends on the mfd maintainer. I would not accept it, as I see it as unnecessary, adding complexity, and pointless. Sure, you can always find examples for everything somewhere in the kernel, but that doesn't make it better.

I'm inclined to agree with Guenter, this patch is just noise.

Ok no problem, i will fix it and resend the patch. For now I know it for the future :-)

Any other comments about the patch series?

If you submit a v2, I'll re-review the bits you leave in.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author maintainer
#8 lore raw

PATCH 2/3 drivers/watchdog/menf21bmc_wd: introduce MEN 14F021P00 BMC Watchdog driver

In reply to Andreas Werner

Added driver to support the 14F021P00 BMC Watchdog.
The BMC is a Board Management Controller including watchdog functionality.

This driver use the I2C interface to the BMC using the menf21bmc MFD Core driver.

---
 drivers/watchdog/Kconfig        |   7 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/menf21bmc_wd.c | 263 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 271 insertions(+)
 create mode 100644 drivers/watchdog/menf21bmc_wd.c
drivers/watchdog/Kconfig +7 -0
index 2b4c1fc..ede3201 100644
@@ -95,6 +95,13 @@ config GPIO_WATCHDOG
 	  If you say yes here you get support for watchdog device
 	  controlled through GPIO-line.
 
+config MENF21BMC_WATCHDOG
+	tristate "MEN 14F021P00 BMC Watchdog"
+	depends on MFD_MENF21BMC
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support for the MEN 14F021P00 BMC Watchdog.
+
 config WM831X_WATCHDOG
 	tristate "WM831x watchdog"
 	depends on MFD_WM831X
drivers/watchdog/Makefile +1 -0
index 1b5f3d5..0a5465d 100644
@@ -178,3 +178,4 @@ obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
 obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
 obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
+obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wd.o
drivers/watchdog/menf21bmc_wd.c new file +263 -0

263 lines not on this page · raw message

Signature · 1 line
1.9.2
maintainer
#9 lore raw

Re: [PATCH 2/3] drivers/watchdog/menf21bmc_wd: introduce MEN 14F021P00 BMC Watchdog driver

Quoted 71 lines · Andreas Werner

Added driver to support the 14F021P00 BMC Watchdog.
The BMC is a Board Management Controller including watchdog functionality.

This driver use the I2C interface to the BMC using the menf21bmc MFD Core driver.

---
  drivers/watchdog/Kconfig        |   7 ++
  drivers/watchdog/Makefile       |   1 +
  drivers/watchdog/menf21bmc_wd.c | 263 ++++++++++++++++++++++++++++++++++++++++
  3 files changed, 271 insertions(+)

create mode 100644 drivers/watchdog/menf21bmc_wd.c

drivers/watchdog/Kconfig +7 -0
index 2b4c1fc..ede3201 100644
@@ -95,6 +95,13 @@ config GPIO_WATCHDOG
  	  If you say yes here you get support for watchdog device
  	  controlled through GPIO-line.
 
+config MENF21BMC_WATCHDOG
+	tristate "MEN 14F021P00 BMC Watchdog"
+	depends on MFD_MENF21BMC
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support for the MEN 14F021P00 BMC Watchdog.
+
  config WM831X_WATCHDOG
  	tristate "WM831x watchdog"
  	depends on MFD_WM831X
drivers/watchdog/Makefile +1 -0
index 1b5f3d5..0a5465d 100644
@@ -178,3 +178,4 @@ obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
  obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
  obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
+obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wd.o
drivers/watchdog/menf21bmc_wd.c new file +25 -0
new file mode 100644
index 0000000..31ccea8
@@ -0,0 +1,263 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC) Watchdog Driver.
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/menf21bmc.h>
+
+#define DEVNAME "menf21bmc_wd"
+
+#define BMC_CMD_WD_ON		0x11
+#define BMC_CMD_WD_OFF		0x12

drivers/watchdog/menf21bmc_wd.c line 35

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+#define BMC_CMD_WD_TRIG		0x13
+#define BMC_CMD_WD_TIME		0x14
+#define BMC_CMD_WD_STATE	0x17
+#define BMC_CMD_WD_ARM_SET	0x18
+#define BMC_CMD_WD_ARM_GET	0x19
+#define BMC_CMD_ARM_STATE	0x19
+#define BMC_WD_OFF_VAL		0x69
+#define BMC_CMD_RST_RSN		0x92
+
+#define BMC_WD_TIMEOUT_MIN	1	/*  in sec -> BMC min = 0.1 s */

I find the comment a bit confusing. The timeout is in seconds. The BMC minimum timeout may be 0.1s, but that is not reflected in the define. Maybe replace the comment with something like

					/* in sec (BMC min = 0.1 s) */
Quoted 13 lines
drivers/watchdog/menf21bmc_wd.c new file +13 -0
+#define BMC_WD_TIMEOUT_MAX	6553	/*  in sec -> BMC max = 6553,5 s */
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+struct menf21bmc_wd_data {
+	struct watchdog_device wdt;
+	struct menf21bmc *menf21bmc;
+};
+
+static int menf21bmc_wd_set_bootstatus(struct menf21bmc_wd_data *data)

drivers/watchdog/menf21bmc_wd.c line 58

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+{
+	u8 rst_rsn;
+	int ret;
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	ret = menf21bmc->read_byte_data(menf21bmc->client,
+						BMC_CMD_RST_RSN, &rst_rsn);
+	if (ret < 0)
+		return -EIO;
+

Please return the original error.

Quoted 14 lines
drivers/watchdog/menf21bmc_wd.c new file +14 -0
+	if (rst_rsn == 0x02)
+		data->wdt.bootstatus |= WDIOF_CARDRESET;
+	else if (rst_rsn == 0x05)
+		data->wdt.bootstatus |= WDIOF_EXTERN1;
+	else if (rst_rsn == 0x06)
+		data->wdt.bootstatus |= WDIOF_EXTERN2;
+	else if (rst_rsn == 0x0A)
+		data->wdt.bootstatus |= WDIOF_POWERUNDER;
+
+	return 0;
+}
+
+static int menf21bmc_wd_leave_prod_mode(struct platform_device *pdev)
+{

drivers/watchdog/menf21bmc_wd.c line 82

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+	int ret;
+	uint8_t val;
+	struct menf21bmc_wd_data *data = platform_get_drvdata(pdev);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	ret = menf21bmc->read_byte_data(menf21bmc->client,
+						BMC_CMD_WD_ARM_GET, &val);
+	if (ret < 0)
+		return -EIO;
+

Please return the original error.

drivers/watchdog/menf21bmc_wd.c line 95

Quoted 13 lines
drivers/watchdog/menf21bmc_wd.c new file +13 -0
+	/*
+	 * Production mode should be not active after delivery of the Board.
+	 * To be sure we check it, inform the user and leave the mode
+	 * if active.
+	 */
+	if (val == 0x00) {
+		dev_info(&pdev->dev,
+			"BMC in production mode. Leave production mode\n");
+
+		ret = menf21bmc->write_byte(menf21bmc->client,
+							BMC_CMD_WD_ARM_SET);
+		if (ret < 0)
+			return -EIO;

Please return the original error.

Quoted 29 lines
drivers/watchdog/menf21bmc_wd.c new file +29 -0
+	}
+
+	return 0;
+}
+
+static int menf21bmc_wd_start(struct watchdog_device *wdt)
+{
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	return menf21bmc->write_byte(menf21bmc->client, BMC_CMD_WD_ON);
+}
+
+static int menf21bmc_wd_stop(struct watchdog_device *wdt)
+{
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	return menf21bmc->write_byte_data(menf21bmc->client,
+					BMC_CMD_WD_OFF, BMC_WD_OFF_VAL);
+}
+
+static int
+menf21bmc_wd_settimeout(struct watchdog_device *wdt, unsigned int timeout)
+{
+	int ret;
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+

drivers/watchdog/menf21bmc_wd.c line 134

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+	/*
+	 *  BMC Watchdog does have a resolution of 100ms.
+	 *  Watchdog API defines the timeout in seconds, so we have to
+	 *  multiply the value.
+	 */
+	ret = menf21bmc->write_word_data(menf21bmc->client,
+						BMC_CMD_WD_TIME, timeout * 10);
+	if (ret < 0)
+		return -EIO;
+

Please return the original error.

Quoted 38 lines
drivers/watchdog/menf21bmc_wd.c new file +38 -0
+	wdt->timeout = timeout;
+
+	return 0;
+}
+
+static int menf21bmc_wd_ping(struct watchdog_device *wdt)
+{
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	return menf21bmc->write_byte(menf21bmc->client, BMC_CMD_WD_TRIG);
+}
+
+static const struct watchdog_info menf21bmc_wd_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+	.identity = DEVNAME,
+};
+
+static const struct watchdog_ops menf21bmc_wd_ops = {
+	.owner		= THIS_MODULE,
+	.start		= menf21bmc_wd_start,
+	.stop		= menf21bmc_wd_stop,
+	.ping		= menf21bmc_wd_ping,
+	.set_timeout	= menf21bmc_wd_settimeout,
+};
+
+static int menf21bmc_wd_probe(struct platform_device *pdev)
+{
+	int ret;
+	uint16_t bmc_timeout;
+	struct menf21bmc_wd_data *driver_data;
+	struct watchdog_device *menf21bmc_wd;
+	struct menf21bmc *menf21bmc = dev_get_drvdata(pdev->dev.parent);
+
+	driver_data = devm_kzalloc(&pdev->dev,
+				sizeof(struct menf21bmc_wd_data), GFP_KERNEL);
+	if (!driver_data)
+		return -ENOMEM;

drivers/watchdog/menf21bmc_wd.c line 182

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+
+	driver_data->menf21bmc = menf21bmc;
+
+	menf21bmc_wd = &driver_data->wdt;
+	menf21bmc_wd->ops = &menf21bmc_wd_ops;
+	menf21bmc_wd->info = &menf21bmc_wd_info;
+	menf21bmc_wd->min_timeout = BMC_WD_TIMEOUT_MIN;
+	menf21bmc_wd->max_timeout = BMC_WD_TIMEOUT_MAX;
+
+

Please no more than one empty line.

Quoted 17 lines
drivers/watchdog/menf21bmc_wd.c new file +17 -0
+	/*
+	 * Get the current wdt timeout value from the BMC because
+	 * the BMC will save the value set before if the system restarts.
+	 */
+	ret = menf21bmc->read_word_data(menf21bmc->client,
+					 BMC_CMD_WD_TIME, &bmc_timeout);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get current WDT timeout\n");
+		return ret;
+	}
+
+	watchdog_init_timeout(menf21bmc_wd, bmc_timeout / 10, &pdev->dev);
+
+	watchdog_set_nowayout(menf21bmc_wd, nowayout);
+	watchdog_set_drvdata(menf21bmc_wd, driver_data);
+	platform_set_drvdata(pdev, driver_data);
+

drivers/watchdog/menf21bmc_wd.c line 209

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+	/*
+	 * We have to leave the Production Mode of the BMC to activate the
+	 * Watchdog functionality.
+	 */
+	ret = menf21bmc_wd_leave_prod_mode(pdev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to leave production mode\n");
+		return ret;
+	}
+

Shouldn't this be in the mfd code ?

Quoted 55 lines
drivers/watchdog/menf21bmc_wd.c new file +54 -0
+	ret = menf21bmc_wd_set_bootstatus(driver_data);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to set Watchdog bootstatus\n");
+		return ret;
+	}
+
+	ret = watchdog_register_device(&driver_data->wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register Watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "MEN 14F021P00 BMC Watchdog device enabled\n");
+
+	return 0;
+}
+
+static int menf21bmc_wd_remove(struct platform_device *pdev)
+{
+	struct menf21bmc_wd_data *data = platform_get_drvdata(pdev);
+
+	dev_warn(&pdev->dev,
+		"Untregister MEN 14F021P00 BMC Watchdog device, board may reset\n");
+
+	watchdog_unregister_device(&data->wdt);
+
+	return 0;
+}
+
+static void menf21bmc_wd_shutdown(struct platform_device *pdev)
+{
+	struct menf21bmc_wd_data *data = platform_get_drvdata(pdev);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	menf21bmc->write_word_data(menf21bmc->client,
+					BMC_CMD_WD_OFF, BMC_WD_OFF_VAL);
+}
+
+static struct  platform_driver menf21bmc_wd = {
+	.driver		= {
+		.owner = THIS_MODULE,
+		.name	= DEVNAME,
+	},
+	.probe		= menf21bmc_wd_probe,
+	.remove		= menf21bmc_wd_remove,
+	.shutdown	= menf21bmc_wd_shutdown,
+};
+
+module_platform_driver(menf21bmc_wd);
+
+MODULE_DESCRIPTION("MEN 14F021P00 BMC Watchdog driver");
+MODULE_AUTHOR("Andreas Werner <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:menf21bmc_wd");
author maintainer
#10 lore raw

Re: [PATCH 2/3] drivers/watchdog/menf21bmc_wd: introduce MEN 14F021P00 BMC Watchdog driver

Quoted 86 lines · Guenter Roeck
Quoted 71 lines · Andreas Werner

Added driver to support the 14F021P00 BMC Watchdog.
The BMC is a Board Management Controller including watchdog functionality.

This driver use the I2C interface to the BMC using the menf21bmc MFD Core driver.

---
 drivers/watchdog/Kconfig        |   7 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/menf21bmc_wd.c | 263 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 271 insertions(+)
 create mode 100644 drivers/watchdog/menf21bmc_wd.c
drivers/watchdog/Kconfig +7 -0
index 2b4c1fc..ede3201 100644
@@ -95,6 +95,13 @@ config GPIO_WATCHDOG
 	  If you say yes here you get support for watchdog device
 	  controlled through GPIO-line.
 
+config MENF21BMC_WATCHDOG
+	tristate "MEN 14F021P00 BMC Watchdog"
+	depends on MFD_MENF21BMC
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support for the MEN 14F021P00 BMC Watchdog.
+
 config WM831X_WATCHDOG
 	tristate "WM831x watchdog"
 	depends on MFD_WM831X
drivers/watchdog/Makefile +1 -0
index 1b5f3d5..0a5465d 100644
@@ -178,3 +178,4 @@ obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
 obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
 obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
+obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wd.o
drivers/watchdog/menf21bmc_wd.c new file +25 -0
new file mode 100644
index 0000000..31ccea8
@@ -0,0 +1,263 @@
+/*
+ *  MEN 14F021P00 Board Management Controller (BMC) Watchdog Driver.
+ *
+ *  Copyright (C) 2014 MEN Mikro Elektronik Nuernberg GmbH
+ *  Author: Andreas Werner <[email protected]>
+ *  All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/menf21bmc.h>
+
+#define DEVNAME "menf21bmc_wd"
+
+#define BMC_CMD_WD_ON		0x11
+#define BMC_CMD_WD_OFF		0x12

drivers/watchdog/menf21bmc_wd.c line 35

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+#define BMC_CMD_WD_TRIG		0x13
+#define BMC_CMD_WD_TIME		0x14
+#define BMC_CMD_WD_STATE	0x17
+#define BMC_CMD_WD_ARM_SET	0x18
+#define BMC_CMD_WD_ARM_GET	0x19
+#define BMC_CMD_ARM_STATE	0x19
+#define BMC_WD_OFF_VAL		0x69
+#define BMC_CMD_RST_RSN		0x92
+
+#define BMC_WD_TIMEOUT_MIN	1	/*  in sec -> BMC min = 0.1 s */

I find the comment a bit confusing. The timeout is in seconds. The BMC minimum timeout may be 0.1s, but that is not reflected in the define. Maybe replace the comment with something like

Quoted 1 line
					/* in sec (BMC min = 0.1 s) */

OK. I think I just write /* in sec */ as the comment because it is explained in menf21bmc_wd_settimout() function.

Quoted 17 lines
Quoted 17 lines
drivers/watchdog/menf21bmc_wd.c new file +16 -0
+#define BMC_WD_TIMEOUT_MAX	6553	/*  in sec -> BMC max = 6553,5 s */
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+struct menf21bmc_wd_data {
+	struct watchdog_device wdt;
+	struct menf21bmc *menf21bmc;
+};
+
+static int menf21bmc_wd_set_bootstatus(struct menf21bmc_wd_data *data)
+{
+	u8 rst_rsn;
+	int ret;
Quoted 8 lines

drivers/watchdog/menf21bmc_wd.c line 58

Quoted 7 lines
drivers/watchdog/menf21bmc_wd.c new file +7 -0
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	ret = menf21bmc->read_byte_data(menf21bmc->client,
+						BMC_CMD_RST_RSN, &rst_rsn);
+	if (ret < 0)
+		return -EIO;
+

Please return the original error.

I agree, will be changed.

Quoted 18 lines
Quoted 18 lines
drivers/watchdog/menf21bmc_wd.c new file +17 -0
+	if (rst_rsn == 0x02)
+		data->wdt.bootstatus |= WDIOF_CARDRESET;
+	else if (rst_rsn == 0x05)
+		data->wdt.bootstatus |= WDIOF_EXTERN1;
+	else if (rst_rsn == 0x06)
+		data->wdt.bootstatus |= WDIOF_EXTERN2;
+	else if (rst_rsn == 0x0A)
+		data->wdt.bootstatus |= WDIOF_POWERUNDER;
+
+	return 0;
+}
+
+static int menf21bmc_wd_leave_prod_mode(struct platform_device *pdev)
+{
+	int ret;
+	uint8_t val;
+	struct menf21bmc_wd_data *data = platform_get_drvdata(pdev);
Quoted 8 lines

drivers/watchdog/menf21bmc_wd.c line 82

Quoted 7 lines
drivers/watchdog/menf21bmc_wd.c new file +7 -0
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	ret = menf21bmc->read_byte_data(menf21bmc->client,
+						BMC_CMD_WD_ARM_GET, &val);
+	if (ret < 0)
+		return -EIO;
+

Please return the original error.

I agree, will be changed.

Quoted 16 lines
Quoted 4 lines
drivers/watchdog/menf21bmc_wd.c new file +3 -0
+	/*
+	 * Production mode should be not active after delivery of the Board.
+	 * To be sure we check it, inform the user and leave the mode

drivers/watchdog/menf21bmc_wd.c line 95

Quoted 10 lines
drivers/watchdog/menf21bmc_wd.c new file +10 -0
+	 * if active.
+	 */
+	if (val == 0x00) {
+		dev_info(&pdev->dev,
+			"BMC in production mode. Leave production mode\n");
+
+		ret = menf21bmc->write_byte(menf21bmc->client,
+							BMC_CMD_WD_ARM_SET);
+		if (ret < 0)
+			return -EIO;

Please return the original error.

I agree, will be changed.

Quoted 33 lines
Quoted 33 lines
drivers/watchdog/menf21bmc_wd.c new file +32 -0
+	}
+
+	return 0;
+}
+
+static int menf21bmc_wd_start(struct watchdog_device *wdt)
+{
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	return menf21bmc->write_byte(menf21bmc->client, BMC_CMD_WD_ON);
+}
+
+static int menf21bmc_wd_stop(struct watchdog_device *wdt)
+{
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	return menf21bmc->write_byte_data(menf21bmc->client,
+					BMC_CMD_WD_OFF, BMC_WD_OFF_VAL);
+}
+
+static int
+menf21bmc_wd_settimeout(struct watchdog_device *wdt, unsigned int timeout)
+{
+	int ret;
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	/*
+	 *  BMC Watchdog does have a resolution of 100ms.
+	 *  Watchdog API defines the timeout in seconds, so we have to
Quoted 8 lines

drivers/watchdog/menf21bmc_wd.c line 134

Quoted 7 lines
drivers/watchdog/menf21bmc_wd.c new file +7 -0
+	 *  multiply the value.
+	 */
+	ret = menf21bmc->write_word_data(menf21bmc->client,
+						BMC_CMD_WD_TIME, timeout * 10);
+	if (ret < 0)
+		return -EIO;
+

Please return the original error.

I agree, will be changed.

Quoted 42 lines
Quoted 42 lines
drivers/watchdog/menf21bmc_wd.c new file +41 -0
+	wdt->timeout = timeout;
+
+	return 0;
+}
+
+static int menf21bmc_wd_ping(struct watchdog_device *wdt)
+{
+	struct menf21bmc_wd_data *data = watchdog_get_drvdata(wdt);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	return menf21bmc->write_byte(menf21bmc->client, BMC_CMD_WD_TRIG);
+}
+
+static const struct watchdog_info menf21bmc_wd_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+	.identity = DEVNAME,
+};
+
+static const struct watchdog_ops menf21bmc_wd_ops = {
+	.owner		= THIS_MODULE,
+	.start		= menf21bmc_wd_start,
+	.stop		= menf21bmc_wd_stop,
+	.ping		= menf21bmc_wd_ping,
+	.set_timeout	= menf21bmc_wd_settimeout,
+};
+
+static int menf21bmc_wd_probe(struct platform_device *pdev)
+{
+	int ret;
+	uint16_t bmc_timeout;
+	struct menf21bmc_wd_data *driver_data;
+	struct watchdog_device *menf21bmc_wd;
+	struct menf21bmc *menf21bmc = dev_get_drvdata(pdev->dev.parent);
+
+	driver_data = devm_kzalloc(&pdev->dev,
+				sizeof(struct menf21bmc_wd_data), GFP_KERNEL);
+	if (!driver_data)
+		return -ENOMEM;
+
+	driver_data->menf21bmc = menf21bmc;
+
Quoted 8 lines

drivers/watchdog/menf21bmc_wd.c line 182

Quoted 7 lines
drivers/watchdog/menf21bmc_wd.c new file +7 -0
+	menf21bmc_wd = &driver_data->wdt;
+	menf21bmc_wd->ops = &menf21bmc_wd_ops;
+	menf21bmc_wd->info = &menf21bmc_wd_info;
+	menf21bmc_wd->min_timeout = BMC_WD_TIMEOUT_MIN;
+	menf21bmc_wd->max_timeout = BMC_WD_TIMEOUT_MAX;
+
+

Please no more than one empty line.

arg, i thought I've catched them all but missed one.

Quoted 21 lines
Quoted 21 lines
drivers/watchdog/menf21bmc_wd.c new file +20 -0
+	/*
+	 * Get the current wdt timeout value from the BMC because
+	 * the BMC will save the value set before if the system restarts.
+	 */
+	ret = menf21bmc->read_word_data(menf21bmc->client,
+					 BMC_CMD_WD_TIME, &bmc_timeout);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get current WDT timeout\n");
+		return ret;
+	}
+
+	watchdog_init_timeout(menf21bmc_wd, bmc_timeout / 10, &pdev->dev);
+
+	watchdog_set_nowayout(menf21bmc_wd, nowayout);
+	watchdog_set_drvdata(menf21bmc_wd, driver_data);
+	platform_set_drvdata(pdev, driver_data);
+
+	/*
+	 * We have to leave the Production Mode of the BMC to activate the
+	 * Watchdog functionality.
Quoted 8 lines

drivers/watchdog/menf21bmc_wd.c line 209

Quoted 7 lines
drivers/watchdog/menf21bmc_wd.c new file +7 -0
+	 */
+	ret = menf21bmc_wd_leave_prod_mode(pdev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to leave production mode\n");
+		return ret;
+	}
+

Shouldn't this be in the mfd code ?

Yes and no :-)
The Production mode needs to be left just to activate the watchdog. That is the reason why I've put these code inside of the watchdog driver.

The other thing is, that we leave the prodcution which is related to the whole BMC, so may be it is really better to put this in the mfd core.

I think I will put it in the mfd core.

Quoted 58 lines
Quoted 57 lines
drivers/watchdog/menf21bmc_wd.c new file +54 -0
+	ret = menf21bmc_wd_set_bootstatus(driver_data);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to set Watchdog bootstatus\n");
+		return ret;
+	}
+
+	ret = watchdog_register_device(&driver_data->wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register Watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "MEN 14F021P00 BMC Watchdog device enabled\n");
+
+	return 0;
+}
+
+static int menf21bmc_wd_remove(struct platform_device *pdev)
+{
+	struct menf21bmc_wd_data *data = platform_get_drvdata(pdev);
+
+	dev_warn(&pdev->dev,
+		"Untregister MEN 14F021P00 BMC Watchdog device, board may reset\n");
+
+	watchdog_unregister_device(&data->wdt);
+
+	return 0;
+}
+
+static void menf21bmc_wd_shutdown(struct platform_device *pdev)
+{
+	struct menf21bmc_wd_data *data = platform_get_drvdata(pdev);
+	struct menf21bmc *menf21bmc = data->menf21bmc;
+
+	menf21bmc->write_word_data(menf21bmc->client,
+					BMC_CMD_WD_OFF, BMC_WD_OFF_VAL);
+}
+
+static struct  platform_driver menf21bmc_wd = {
+	.driver		= {
+		.owner = THIS_MODULE,
+		.name	= DEVNAME,
+	},
+	.probe		= menf21bmc_wd_probe,
+	.remove		= menf21bmc_wd_remove,
+	.shutdown	= menf21bmc_wd_shutdown,
+};
+
+module_platform_driver(menf21bmc_wd);
+
+MODULE_DESCRIPTION("MEN 14F021P00 BMC Watchdog driver");
+MODULE_AUTHOR("Andreas Werner <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:menf21bmc_wd");
author maintainer
#11 lore raw

PATCH 3/3 drivers/leds/leds-menf21bmc: introduce MEN 14F021P00 BMC LED driver

In reply to Andreas Werner

Added driver to support the 14F021P00 BMC LEDs.
The BMC is a Board Management Controll include four LEDs which can be switched on and off.

This driver use the I2C interface to the BMC using the menf21bmc MFD Core driver.

---
 drivers/leds/Kconfig          |   6 ++
 drivers/leds/Makefile         |   2 +-
 drivers/leds/leds-menf21bmc.c | 135 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-menf21bmc.c
drivers/leds/Kconfig +6 -0
index 1e81d7a..8f98e0f 100644
@@ -466,6 +466,12 @@ config LEDS_OT200
 	  This option enables support for the LEDs on the Bachmann OT200.
 	  Say Y to enable LEDs on the Bachmann OT200.
 
+config LEDS_MENF21BMC
+	tristate "LED support for the MEN 14F021P00 BMC"
+	depends on LEDS_CLASS && MFD_MENF21BMC
+	help
+	  Say Y here to include support for the MEN 14F021P00 BMC LEDs.
+
 comment "LED driver for blink(1) USB RGB LED is under Special HID drivers (HID_THINGM)"
 
 config LEDS_BLINKM
drivers/leds/Makefile +1 -1
index 79c5155..1c465fe 100644
@@ -54,7 +54,7 @@ obj-$(CONFIG_LEDS_MAX8997)		+= leds-max8997.o
 obj-$(CONFIG_LEDS_LM355x)		+= leds-lm355x.o
 obj-$(CONFIG_LEDS_BLINKM)		+= leds-blinkm.o
 obj-$(CONFIG_LEDS_VERSATILE)		+= leds-versatile.o
-
+obj-$(CONFIG_LEDS_MENF21BMC)		+= leds-menf21bmc.o
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_DAC124S085)		+= leds-dac124s085.o
 
drivers/leds/leds-menf21bmc.c new file +135 -0

135 lines not on this page · raw message

Signature · 1 line
1.9.2