[PATCH linux v1 2/3] drivers: misc: Character driver to update BMC post code
Rick Altherr
raltherr at google.com
Sun Oct 16 03:11:08 AEDT 2016
On Fri, Oct 14, 2016 at 5:00 PM, Jaghathiswari Rankappagounder Natarajan <
jaghu at google.com> wrote:
> By this comment "want this character device to attach to any compatible
> display drivers." , should I have a separate setup_character device
> function(add the platform device(display driver for GPIO) as the parent of
> the character device) and call this function from the corresponding
> display driver for GPIO? So display driver for GPIO will only setup
> this character device ?
>
> I'm getting confused by the terminology here. I would expect a platform
driver that matches on the device tree node, provides an API for displaying
on a 7-segment display, and implements the bit-banging necessary to do so.
The character device driver would match on the previous platform driver and
implement the user-space API for letting a user write to the display
including any conversion methods necessary to map the user input to a
7-segment display.
> I have moved the function to convert character buffer to led display
> pattern to this char driver. The display is very specific for BMC post
> codes( characters '14' is displayed as digits 't4' and the DOT lights up on
> two segment LEDs ) and generic for host POST codes(characters '14' is
> displayed as '14' and DOT doesn't light up). This info is present in the
> lookup table.
> When display driver for GPIO wants to use a character device with
> different conversion(say to display '14' as '14 with DOTs light up) it
> needs a different character device(which does conversion differently) ?
>
> Yes, the conversion should be associated with the character device. My
rationale is that the conversion used depends on how the user provided the
data (free form string, fixed string, integer). If the user API is
different, it should have a different character device so the user-space
app knows what API it needs to implement.
> should the mapping be like this char device(with a specific display
> pattern) can be used with any display driver(GPIO and SGPIO) and the GPIO
> driver/SGPIO driver can use any character device(with different display
> conversion patterns) ? is it better to do as above or keep as 1-1 mapping?
> or keep the conversion logic in BMC/Host side?
>
>
I was thinking of having a subsystem layer that abstracts interfacing with
7-segment displays. It probably isn't necessary today but would make it
simpler to implement other output drivers (using a different shift register
with the Aspeed SGPIO hardware, etc) or implement different user-space APIs.
>
> On Wed, Oct 12, 2016 at 6:15 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:
>>
>>> Character driver - On state change, the BMC will write its POST code to
>>> the
>>> character device. The POST code is then sent to update-postcard platform
>>> driver for display on the POST card. The BMC POST code consists of 3
>>> digits
>>>
>>> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu at google.com
>>> >
>>> ---
>>> drivers/misc/Kconfig | 8 +++
>>> drivers/misc/Makefile | 1 +
>>> drivers/misc/update-bmc-postcode.c | 123 ++++++++++++++++++++++++++++++
>>> +++++++
>>> 3 files changed, 132 insertions(+)
>>> create mode 100644 drivers/misc/update-bmc-postcode.c
>>>
>>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>>> index d65b83f..69dc3b6 100644
>>> --- a/drivers/misc/Kconfig
>>> +++ b/drivers/misc/Kconfig
>>> @@ -815,6 +815,14 @@ config ASPEED_BT_IPMI_HOST
>>> help
>>> Support for the Aspeed BT ipmi host.
>>>
>>> +config ASPEED_UPDATE_BMC_POSTCODE
>>> + tristate "Character driver for BMC POST code update"
>>> + depends on ASPEED_UPDATE_POSTCARD
>>> + help
>>> + On state change, the BMC will write its POST code to the
>>> + character device. The POST code is then sent to update-postcard
>>> platform
>>> + driver for display on the POST card
>>> +
>>>
>>
>> Again, not specific to Aspeed. Maybe this should be
>> SEVEN_SEGMENT_DISPLAY and the other driver should be SEVEN_SEGMENT_SGPIO.
>>
>>
>>> source "drivers/misc/c2port/Kconfig"
>>> source "drivers/misc/eeprom/Kconfig"
>>> source "drivers/misc/cb710/Kconfig"
>>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>>> index 730c6c2..dce20cb 100644
>>> --- a/drivers/misc/Makefile
>>> +++ b/drivers/misc/Makefile
>>> @@ -59,3 +59,4 @@ 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
>>> +obj-$(CONFIG_ASPEED_UPDATE_BMC_POSTCODE) += update-bmc-postcode.o
>>> diff --git a/drivers/misc/update-bmc-postcode.c
>>> b/drivers/misc/update-bmc-postcode.c
>>> new file mode 100644
>>> index 0000000..8e368a4
>>> --- /dev/null
>>> +++ b/drivers/misc/update-bmc-postcode.c
>>> @@ -0,0 +1,123 @@
>>> +/*
>>> + * Character driver - On state change, the BMC will write its POST code
>>> to the
>>> + * character device. The POST code is then sent to update-postcard
>>> platform
>>> + * driver for display on the POST card. The BMC POST code consists of 3
>>> digits.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/version.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/types.h>
>>> +#include <linux/kdev_t.h>
>>> +#include <linux/fs.h>
>>> +#include <linux/device.h>
>>> +#include <linux/cdev.h>
>>> +#include <linux/uaccess.h>
>>> +#include <linux/ctype.h>
>>> +
>>> +#include "update-postcard.h"
>>> +
>>> +#define MAX_POSTCODE_SIZE 3
>>> +
>>> +static dev_t bmc_state_dev;
>>> +static struct cdev c_dev;
>>> +static struct class *bmc_state_class;
>>> +
>>> +static int bmc_state_open(struct inode *i, struct file *f)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int bmc_state_close(struct inode *i, struct file *f)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static ssize_t bmc_state_read(struct file *f, char __user *buf, size_t
>>> + len, loff_t *off)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +/* Write post code from bmc to the correct variable */
>>> +static ssize_t bmc_state_write(struct file *f, const char __user *buf,
>>> + size_t len, loff_t *off)
>>> +{
>>> + char tmp[MAX_POSTCODE_SIZE];
>>> + int length = len - 1;
>>> + int i;
>>> +
>>> + if (length != MAX_POSTCODE_SIZE) {
>>> + return -EINVAL;
>>> + }
>>> +
>>> + if (copy_from_user(tmp, buf, length) != 0) {
>>> + return -EFAULT;
>>> + }
>>> +
>>> + for (i = 0; i < MAX_POSTCODE_SIZE; i++) {
>>> + if (!isxdigit(tmp[i]))
>>> + return -EINVAL;
>>> + }
>>> +
>>> + update_postcode(tmp);
>>>
>>
>> As written, this is hard-coded to the bit-bang driver in your other
>> patch. The concept is fairly generic though. If we switched to using a
>> 74HC165 that is compatible with the Aspeed's built-in SGPIO hardware,
>> ideally we'd only need to write a driver for the SGPIO. That implies you
>> want this character device to attach to any compatible display drivers.
>>
>>
>>> + return len;
>>> +}
>>> +
>>> +static const struct file_operations bmc_state_fops = {
>>> +
>>> + .owner = THIS_MODULE,
>>> + .open = bmc_state_open,
>>> + .release = bmc_state_close,
>>> + .read = bmc_state_read,
>>> + .write = bmc_state_write
>>> +};
>>> +
>>> +static int __init update_bmc_pc_init(void)
>>> +{
>>> + if (alloc_chrdev_region(&bmc_state_dev, 0, 1, "bmc_state") < 0)
>>> {
>>> + return -1;
>>> + }
>>> +
>>> + bmc_state_class = class_create(THIS_MODULE, "bmc_state");
>>> + if (bmc_state_class == NULL) {
>>> + goto unregister;
>>> + return -1;
>>> + }
>>> +
>>> + if (device_create(bmc_state_class, NULL, bmc_state_dev,
>>> + NULL, "current_state") == NULL) {
>>> + goto class_destroy;
>>> + return -1;
>>> + }
>>> +
>>> + cdev_init(&c_dev, &bmc_state_fops);
>>> + if (cdev_add(&c_dev, bmc_state_dev, 1) == -1) {
>>> + goto device_destroy;
>>> + return -1;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +device_destroy:
>>> + device_destroy(bmc_state_class, bmc_state_dev);
>>> +class_destroy:
>>> + class_destroy(bmc_state_class);
>>> +unregister:
>>> + unregister_chrdev_region(bmc_state_dev, 1);
>>> + return -1;
>>> +}
>>> +
>>> +static void __exit update_bmc_pc_exit(void)
>>> +{
>>> + cdev_del(&c_dev);
>>> + device_destroy(bmc_state_class, bmc_state_dev);
>>> + class_destroy(bmc_state_class);
>>> + unregister_chrdev_region(bmc_state_dev, 1);
>>> +}
>>> +
>>> +module_init(update_bmc_pc_init);
>>> +module_exit(update_bmc_pc_exit);
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <
>>> jaghu at google.com>");
>>> +MODULE_DESCRIPTION("Character driver - update bmc postcode");
>>> --
>>> 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/20161015/45cfbbca/attachment.html>
More information about the openbmc
mailing list