[PATCH linux v1 1/3] drivers: misc: Platform driver to update display on the POST card
Jaghathiswari Rankappagounder Natarajan
jaghu at google.com
Wed Nov 2 05:22:04 AEDT 2016
On Wed, Oct 12, 2016 at 6:09 PM, Rick Altherr <raltherr at google.com> wrote:
>
> On Wed, Oct 12, 2016 at 5:41 PM, Jaghathiswari Rankappagounder Natarajan <
> jaghu at google.com> wrote:
>
>> Platform driver to periodically update the display on the POST card.
>> POST card hardware assumed is 74HC164 wired to two 7-segment displays.
>>
>> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu at google.com>
>> ---
>> drivers/misc/Kconfig | 6 ++
>> drivers/misc/Makefile | 1 +
>> drivers/misc/update-postcard.c | 219 ++++++++++++++++++++++++++++++
>> +++++++++++
>> drivers/misc/update-postcard.h | 1 +
>> 4 files changed, 227 insertions(+)
>> create mode 100644 drivers/misc/update-postcard.c
>> create mode 100644 drivers/misc/update-postcard.h
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 4617ddc..d65b83f 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -804,6 +804,12 @@ config PANEL_BOOT_MESSAGE
>> An empty message will only clear the display at driver init
>> time. Any other
>> printf()-formatted message is valid with newline and escape
>> codes.
>>
>> +config ASPEED_UPDATE_POSTCARD
>> + tristate "Platform driver to update POST card"
>> + help
>> + Support to periodically update display on the POST card. The
>> POST card
>> + hardware assumed is 74HC164 wired to two 7-segment displays.
>> +
>>
>
> Nothing about this driver is associated with Aspeed. UPDATE_POSTCARD
> reads to me as describing an action, not a device. Maybe
> SEVEN_SEGMENT_DISPLAY?
>
>
>> config ASPEED_BT_IPMI_HOST
>> tristate "BT IPMI host driver"
>> help
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 724861b..730c6c2 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -57,4 +57,5 @@ obj-$(CONFIG_ECHO) += echo/
>> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
>> obj-$(CONFIG_CXL_BASE) += cxl/
>> obj-$(CONFIG_PANEL) += panel.o
>> +obj-$(CONFIG_ASPEED_UPDATE_POSTCARD) += update-postcard.o
>> obj-$(CONFIG_ASPEED_BT_IPMI_HOST) += bt-host.o
>> diff --git a/drivers/misc/update-postcard.c
>> b/drivers/misc/update-postcard.c
>> new file mode 100644
>> index 0000000..3516c24
>> --- /dev/null
>> +++ b/drivers/misc/update-postcard.c
>> @@ -0,0 +1,219 @@
>> +/* Platform driver to periodically update the display on the POST card.
>> + * POST card hardware assumed is 74HC164 wired to two 7-segment displays.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/timer.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/sizes.h>
>> +#include <linux/map_to_7segment.h>
>> +#include <linux/io.h>
>> +#include <linux/delay.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/gpio/consumer.h>
>> +
>> +#define POST_CODE_UPDATE_INTVL 1000
>> +
>> +#define MAX_PC_SIZE 3
>> +
>> +#define LED_DOT 0x01
>> +
>> +#define CLOCK_GPIO_NAME "clk"
>> +#define DATA_GPIO_NAME "data"
>> +#define CLEAR_GPIO_NAME "clr"
>> +
>> +static struct mutex mutex;
>> +
>> +static struct timer_list update_timer;
>> +
>> +static char curr_postcode[3];
>> +static u8 pc_valid;
>> +
>>
> +static struct gpio_desc *clk_gpio;
>> +static struct gpio_desc *data_gpio;
>> +static struct gpio_desc *clr_gpio;
>>
>
> I realize there should really only ever be one of these in a system but
> static data members in modules bug me. I'd much rather see all this
> configuration and these buffers in a struct that is saved with
> platform_set_drvdata().
>
>
>> +
>> +/*
>> + * 0 1 2 3 4 5 6 7 8 9 A B C D E F
>> + * _ _ _ _ _ _ _ _ _ _ _ _
>> + * | | | _| _| |_| |_ |_ | |_| |_| |_| |_ | _| |_ |_
>> + * |_| | |_ _| | _| |_| | |_| | | | |_| |_ |_| |_ |
>> + *
>> + * data[7:1] = led[a:g]
>> + * lookup table used for both bytes of lpc post code and lower byte of
>> + * bmc post code
>> + */
>> +const u8 seven_seg_bits[] = {
>> + 0xFC, 0x60, 0xDA, 0xF2, 0x66, 0xB6, 0xBE, 0xE0,
>> + 0xFE, 0xF6, 0xEE, 0x3E, 0x9C, 0x7A, 0x9E, 0x8E
>> + };
>> +
>> +/*
>> + * 0 1 2 3 4 5 6 7 8 9 A B C D E F
>> + * _ _ _ _ _
>> + * | |_ |_| |_ _ _ _ _ _ _ _ |_ _| _| | |
>> + * |_ |_ | | _| |_| |_| | |
>> + *
>> + * data[7:1] = led[a:g]
>> + * lookup table used for higher byte of bmc post code
>> + */
>> +const u8 special_seven_seg_bits[] = {
>> + 0x00, 0x9C, 0x1E, 0xCE, 0x8E, 0x02, 0x02, 0x02,
>> + 0x02, 0x02, 0x02, 0x02, 0xB6, 0x7A, 0x7A, 0xEC
>> + };
>> +
>> +void update_postcode(char *buf)
>> +{
>> + mutex_lock(&mutex);
>> + strncpy(curr_postcode, buf, sizeof(curr_postcode));
>> + mutex_unlock(&mutex);
>> + pc_valid = 1;
>> +}
>> +EXPORT_SYMBOL(update_postcode);
>> +
>> +/* convert postcode to led pattern
>> + * 7-bits used for each 7-seg display and
>> + * 1 bit used for the 'dot' on both digits.
>> + *
>> + * @param N/A
>> + * @return N/A
>> + */
>> +static u16 postcode_to_led_signal(void)
>> +{
>> + u8 low_display;
>> + u8 high_display;
>> + u16 led_value;
>> + char buf[3];
>> +
>> + mutex_lock(&mutex);
>> + strncpy(buf, curr_postcode, sizeof(buf));
>> + mutex_unlock(&mutex);
>> +
>> + low_display = seven_seg_bits[hex_to_bin(buf[2])];
>> +
>> + high_display = (buf[0] == '1') ?
>> + special_seven_seg_bits[hex_to_bin(buf[1])] :
>> + seven_seg_bits[hex_to_bin(buf[1])];
>> +
>> + led_value = low_display | (high_display << 8);
>> + if (buf[0] == '1') {
>> + led_value |= LED_DOT | (LED_DOT << 8);
>> + }
>> +
>> + return led_value;
>> +}
>> +
>> +static void update_sgpio(void)
>> +{
>> + u16 led_signal;
>> + int i;
>> +
>> + /* Convert two bytes of post code to 16 bit led pattern */
>> + led_signal = postcode_to_led_signal();
>>
>
> Why do you wait until the display update to convert the string to the LED
> bit pattern? For that matter, why does update_postcode() take a string at
> all? If you can only display at most a 16-bit number, take a u16 and have
> the caller deal with whatever processing they need.
>
>
>> +
>> + /* Set the clear signal to low */
>> + gpiod_set_value(clr_gpio, 0);
>> + udelay(1);
>> + /* Set the clear clear to high */
>> + gpiod_set_value(clr_gpio, 1);
>> + udelay(1);
>> +
>> + /* Bitbang the 16 bit led pattern
>> + * 7-bits used for each 7-seg display
>> + * 1 bit used for the 'dot' on both digits
>> + */
>> + for (i = 0; i < 16; i++) {
>> + if (led_signal & 0x01) {
>> + /* Set the data signal to high */
>> + gpiod_set_value(data_gpio, 1);
>> + } else {
>> + /* Set the data signal to low */
>> + gpiod_set_value(data_gpio, 0);
>> + }
>> + udelay(1);
>> +
>> + /* Set the clock signal to low */
>> + gpiod_set_value(clk_gpio, 0);
>> + udelay(1);
>> + /* Set the clock signal to high */
>> + gpiod_set_value(clk_gpio, 1);
>> + udelay(1);
>> +
>> + led_signal >>= 1;
>> + }
>> +}
>> +
>> +static void update_timer_handler(unsigned long data)
>> +{
>> + if (pc_valid == 1) {
>>
>
> Is this to avoid the time after the driver is register until
> update_postcode() is called the first time? Is there an API for clearing
> the display?
>
Yes, this is to wait until the user gives a valid display data. I have
included an API for clearing the display in the next patch.
>
>
>> + update_sgpio();
>> + }
>> + mod_timer(&update_timer,
>> + jiffies + msecs_to_jiffies(POST_CODE_UPDATE_INTVL));
>> +}
>> +
>> +static const struct of_device_id of_postcard_match[] = {
>> + { .compatible = "postcard-display" },
>> + {},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, of_postcard_match);
>> +
>> +static int postcard_probe(struct platform_device *pdev)
>> +{
>> + int result;
>> + struct device *dev = &pdev->dev;
>> +
>> + /* Requesting the clock gpio */
>> + clk_gpio = devm_gpiod_get(dev, CLOCK_GPIO_NAME,
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(clk_gpio))
>> + return PTR_ERR(data_gpio);
>> +
>> + /* Requesting the data gpio */
>> + data_gpio = devm_gpiod_get(dev, DATA_GPIO_NAME,
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(data_gpio))
>> + return PTR_ERR(data_gpio);
>> +
>> + /* Requesting the clear gpio */
>> + clr_gpio = devm_gpiod_get(dev, CLEAR_GPIO_NAME,
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(clr_gpio))
>> + return PTR_ERR(clr_gpio);
>> +
>> + /* Start timer to update post code every second */
>> + setup_timer(&update_timer, update_timer_handler, 0);
>> + result = mod_timer(&update_timer,
>> + jiffies + msecs_to_jiffies(POST_CODE_UPDATE_INTVL));
>> +
>> + if (result)
>> + return result;
>> +
>> + mutex_init(&mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static void postcard_remove(struct platform_device *pdev)
>> +{
>> + del_timer(&update_timer);
>> +}
>> +
>> +static struct platform_driver postcard_display_driver = {
>> + .probe = postcard_probe,
>> + .shutdown = postcard_remove,
>> + .driver = {
>> + .name = "postcard-display",
>> + .of_match_table = of_postcard_match,
>> + },
>> +};
>> +
>> +module_platform_driver(postcard_display_driver);
>
> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu at google.com
>> >");
>> +MODULE_DESCRIPTION("Post card display update driver");
>> diff --git a/drivers/misc/update-postcard.h
>> b/drivers/misc/update-postcard.h
>> new file mode 100644
>> index 0000000..ef82e9091d
>> --- /dev/null
>> +++ b/drivers/misc/update-postcard.h
>> @@ -0,0 +1 @@
>> +void update_postcode(char *buf);
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>> _______________________________________________
>> openbmc mailing list
>> openbmc at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/openbmc
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20161101/e937a51e/attachment-0001.html>
More information about the openbmc
mailing list