From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750848AbdKEPio (ORCPT ); Sun, 5 Nov 2017 10:38:44 -0500 Received: from mail-lf0-f65.google.com ([209.85.215.65]:49781 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750731AbdKEPim (ORCPT ); Sun, 5 Nov 2017 10:38:42 -0500 X-Google-Smtp-Source: ABhQp+QDxetoRFG2ZTracDNk7tjjVuo3ZweZIO/UpZAwNcFKy7hZYQ2mLVw68Ob0b7wB8mUjf5aYKg== Date: Sun, 5 Nov 2017 16:38:38 +0100 From: Johan Hovold To: Andrey Smirnov Cc: linux-kernel@vger.kernel.org, cphealy@gmail.com, Lucas Stach , Nikita Yushchenko , Lee Jones , Greg Kroah-Hartman , Pavel Machek , Andy Shevchenko , Guenter Roeck , Rob Herring , Johan Hovold , Sebastian Reichel Subject: Re: [PATCH v10 3/5] mfd: Add driver for RAVE Supervisory Processor Message-ID: <20171105153838.GB13118@localhost> References: <20171031163656.24552-1-andrew.smirnov@gmail.com> <20171031163656.24552-4-andrew.smirnov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171031163656.24552-4-andrew.smirnov@gmail.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? > +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). > + 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+). > +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. > 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/ > @@ -0,0 +1,54 @@ > +#ifndef _LINUX_RAVE_SP_H_ > +#define _LINUX_RAVE_SP_H_ > + > +enum rave_sp_command { > + RAVE_SP_CMD_GET_FIRMWARE_VERSION = 0x20, > + RAVE_SP_CMD_GET_BOOTLOADER_VERSION = 0x21, > + RAVE_SP_CMD_BOOT_SOURCE = 0x26, > + RAVE_SP_CMD_GET_BOARD_COPPER_REV = 0x2B, > + RAVE_SP_CMD_GET_GPIO_STATE = 0x2F, > + > + RAVE_SP_CMD_STATUS = 0xA0, > + RAVE_SP_CMD_SW_WDT = 0xA1, > + RAVE_SP_CMD_PET_WDT = 0xA2, > + RAVE_SP_CMD_RESET = 0xA7, > + RAVE_SP_CMD_RESET_REASON = 0xA8, > + > + RAVE_SP_CMD_REQ_COPPER_REV = 0xB6, > + RAVE_SP_CMD_GET_I2C_DEVICE_STATUS = 0xBA, > + RAVE_SP_CMD_GET_SP_SILICON_REV = 0xB9, > + RAVE_SP_CMD_CONTROL_EVENTS = 0xBB, > + > + RAVE_SP_EVNT_BASE = 0xE0, > +}; Johan