Messages in this thread | | | From | Andrey Smirnov <> | Date | Sun, 5 Nov 2017 14:02:00 -0800 | Subject | Re: [PATCH v10 3/5] mfd: Add driver for RAVE Supervisory Processor |
| |
On Sun, Nov 5, 2017 at 7:38 AM, Johan Hovold <johan@kernel.org> wrote: > On Tue, Oct 31, 2017 at 09:36:54AM -0700, Andrey Smirnov wrote: >> Add a driver for RAVE Supervisory Processor, an MCU implementing >> various bits of housekeeping functionality (watchdoging, backlight >> control, LED control, etc) on RAVE family of products by Zodiac >> Inflight Innovations. >> >> This driver implementes core MFD/serdev device as well as >> communication subroutines necessary for commanding the device. >> >> Cc: linux-kernel@vger.kernel.org >> Cc: cphealy@gmail.com >> Cc: Lucas Stach <l.stach@pengutronix.de> >> Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com> >> Cc: Lee Jones <lee.jones@linaro.org> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Pavel Machek <pavel@ucw.cz> >> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> >> Cc: Guenter Roeck <linux@roeck-us.net> >> Cc: Rob Herring <robh@kernel.org> >> Cc: Johan Hovold <johan@kernel.org> >> Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk> >> Tested-by: Chris Healy <cphealy@gmail.com> >> Reviewed-by: Guenter Roeck <linux@roeck-us.net> >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> >> --- > >> +int rave_sp_exec(struct rave_sp *sp, >> + void *__data, size_t data_size, >> + void *reply_data, size_t reply_data_size) >> +{ >> + struct rave_sp_reply reply = { >> + .data = reply_data, >> + .length = reply_data_size, >> + .received = COMPLETION_INITIALIZER_ONSTACK(reply.received), >> + }; >> + unsigned char *data = __data; >> + int command, ret = 0; >> + u8 ackid; >> + >> + command = sp->variant->cmd.translate(data[0]); >> + if (command < 0) >> + return command; >> + >> + ackid = atomic_inc_return(&sp->ackid); >> + reply.ackid = ackid; >> + reply.code = rave_sp_reply_code((u8)command), >> + >> + mutex_lock(&sp->bus_lock); >> + >> + mutex_lock(&sp->reply_lock); >> + sp->reply = &reply; >> + mutex_unlock(&sp->reply_lock); >> + >> + data[0] = command; >> + data[1] = ackid; >> + >> + rave_sp_write(sp, data, data_size); >> + >> + if (!wait_for_completion_timeout(&reply.received, HZ)) { >> + dev_err(&sp->serdev->dev, "Command timeout\n"); >> + ret = -ETIMEDOUT; >> + >> + mutex_lock(&sp->reply_lock); >> + sp->reply = NULL; >> + mutex_unlock(&sp->reply_lock); >> + } >> + >> + mutex_unlock(&sp->bus_lock); >> + return ret; >> +} >> +EXPORT_SYMBOL(rave_sp_exec); > > EXPORT_SYMBOL_GPL? >
Yep, will change in v11.
>> +static int rave_sp_receive_buf(struct serdev_device *serdev, >> + const unsigned char *buf, size_t size) >> +{ >> + struct device *dev = &serdev->dev; >> + struct rave_sp *sp = dev_get_drvdata(dev); >> + struct rave_sp_deframer *deframer = &sp->deframer; >> + const unsigned char *src = buf; >> + const unsigned char *end = buf + size; >> + bool reset_framer = false; >> + >> + while (src < end) { >> + const unsigned char byte = *src++; >> + >> + switch (deframer->state) { >> + case RAVE_SP_EXPECT_SOF: >> + if (byte == RAVE_SP_STX) >> + deframer->state = RAVE_SP_EXPECT_DATA; >> + continue; >> + >> + case RAVE_SP_EXPECT_DATA: >> + switch (byte) { >> + case RAVE_SP_ETX: >> + rave_sp_receive_frame(sp, >> + deframer->data, >> + deframer->length); >> + reset_framer = true; >> + break; >> + case RAVE_SP_STX: >> + dev_warn(dev, "Bad frame: STX before ETX\n"); >> + reset_framer = true; >> + break; >> + case RAVE_SP_DLE: >> + deframer->state = RAVE_SP_EXPECT_ESCAPED_DATA; >> + continue; >> + } >> + >> + case RAVE_SP_EXPECT_ESCAPED_DATA: /* FALLTHROUGH */ > > The fallthrough comment should precede the case statement (or you will > soon receive a patch from the people working enabling > -Wimplicit-fallthrough). >
Sure, will change in v11.
>> + deframer->data[deframer->length++] = byte; >> + >> + if (deframer->length == sizeof(deframer->data)) { >> + dev_warn(dev, "Bad frame: Too long\n"); >> + reset_framer = true; >> + break; >> + } >> + >> + deframer->state = RAVE_SP_EXPECT_DATA; >> + break; >> + } >> + } >> + >> + if (reset_framer) { >> + deframer->state = RAVE_SP_EXPECT_SOF; >> + deframer->length = 0; >> + } >> + >> + return src - buf; >> +} > >> +MODULE_LICENSE("GPL v2"); > > This doesn't match the license text (GPL-2.0+). >
Will update in v11.
>> +MODULE_AUTHOR("Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>"); >> +MODULE_AUTHOR("Nikita Yushchenko <nikita.yoush@cogentembedded.com>"); >> +MODULE_AUTHOR("Andrey Smirnov <andrew.smirnov@gmail.com>"); >> +MODULE_DESCRIPTION("RAVE SP core driver"); >> diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig >> index c11db8bceea1..989d38b587bf 100644 >> --- a/drivers/platform/Kconfig >> +++ b/drivers/platform/Kconfig >> @@ -8,3 +8,4 @@ endif >> source "drivers/platform/goldfish/Kconfig" >> >> source "drivers/platform/chrome/Kconfig" >> + > > This chunk should not be here. >
Missed this one. Will remove in v11.
>> diff --git a/include/linux/rave-sp.h b/include/linux/rave-sp.h >> new file mode 100644 >> index 000000000000..811929afc1a9 >> --- /dev/null >> +++ b/include/linux/rave-sp.h > > I think this one should live in include/linux/mfd/
My bad, forgot to move that file. Will do in v11.
Thanks, Andrey Smirnov
| |