From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751307AbdKEWCD (ORCPT ); Sun, 5 Nov 2017 17:02:03 -0500 Received: from mail-qk0-f196.google.com ([209.85.220.196]:47796 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750769AbdKEWCB (ORCPT ); Sun, 5 Nov 2017 17:02:01 -0500 X-Google-Smtp-Source: ABhQp+Ql/8T9kAU6bdDq4JdWeMmzrBY9kOJHy8nGdowkHnJ/2cunczm7Inl21mBAPob3sDsBRXDTxtaaj+IiEldDEgo= MIME-Version: 1.0 In-Reply-To: <20171105153838.GB13118@localhost> References: <20171031163656.24552-1-andrew.smirnov@gmail.com> <20171031163656.24552-4-andrew.smirnov@gmail.com> <20171105153838.GB13118@localhost> From: Andrey Smirnov Date: Sun, 5 Nov 2017 14:02:00 -0800 Message-ID: Subject: Re: [PATCH v10 3/5] mfd: Add driver for RAVE Supervisory Processor To: Johan Hovold Cc: linux-kernel , Chris Healy , Lucas Stach , Nikita Yushchenko , Lee Jones , Greg Kroah-Hartman , Pavel Machek , Andy Shevchenko , Guenter Roeck , Rob Herring , Sebastian Reichel Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Nov 5, 2017 at 7:38 AM, Johan Hovold 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 >> Cc: Nikita Yushchenko >> Cc: Lee Jones >> Cc: Greg Kroah-Hartman >> Cc: Pavel Machek >> Cc: Andy Shevchenko >> Cc: Guenter Roeck >> Cc: Rob Herring >> Cc: Johan Hovold >> Cc: Sebastian Reichel >> Tested-by: Chris Healy >> Reviewed-by: Guenter Roeck >> Reviewed-by: Andy Shevchenko >> Signed-off-by: Andrey Smirnov >> --- > >> +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 "); >> +MODULE_AUTHOR("Nikita Yushchenko "); >> +MODULE_AUTHOR("Andrey Smirnov "); >> +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