Re: [PATCH 1/3] mfd: rave-sp: Add code to print firmware versions

[Lucas Stach]

Am Montag, den 26.02.2018, 07:07 -0800 schrieb Andrey Smirnov:
> Add code that would query and print out bootloader and application
> firmware version info.
> 
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: cphealy@xxxxxxxxx
> Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Cc: Lee Jones <lee.jones@xxxxxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>

Tested-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>

> ---
> 
> Lee:
> 
> The reason 'part_number_firmware' and 'part_number_firmware' are a
> part of struct rave_sp is because there exists another patch on top
> of
> this one that exposes those fields via sysfs. The latter patch is not
> currently being upstreamed (might be in the future), so if keeping
> this arrangement is too ugly, let me know, and I'll get rid of those
> fields in 'rave_sp'.
> 
> Thanks,
> Andrey Smirnov
> 
> 
> Âdrivers/mfd/rave-sp.c | 97
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> Â1 file changed, 97 insertions(+)
> 
> diff --git a/drivers/mfd/rave-sp.c b/drivers/mfd/rave-sp.c
> index c8173de5653a..9e4c83ff2aec 100644
> --- a/drivers/mfd/rave-sp.c
> +++ b/drivers/mfd/rave-sp.c
> @@ -160,6 +160,8 @@ struct rave_sp_variant {
> Â * @variant:			Device variant specific
> information
> Â * @event_notifier_list:	Input event notification chain
> Â *
> + * @part_number_firmware:	Firmware version
> + * @part_number_bootloader:	Bootloader version
> Â */
> Âstruct rave_sp {
> Â	struct serdev_device *serdev;
> @@ -171,8 +173,42 @@ struct rave_sp {
> Â
> Â	const struct rave_sp_variant *variant;
> Â	struct blocking_notifier_head event_notifier_list;
> +
> +	const char *part_number_firmware;
> +	const char *part_number_bootloader;
> Â};
> Â
> +struct rave_sp_version {
> +	u8ÂÂÂÂÂhardware;
> +	__le16 major;
> +	u8ÂÂÂÂÂminor;
> +	u8ÂÂÂÂÂletter[2];
> +} __packed;
> +
> +struct rave_sp_status {
> +	struct rave_sp_version bootloader_version;
> +	struct rave_sp_version firmware_version;
> +	u16 rdu_eeprom_flag;
> +	u16 dds_eeprom_flag;
> +	u8ÂÂpic_flag;
> +	u8ÂÂorientation;
> +	u32 etc;
> +	s16 temp[2];
> +	u8ÂÂbacklight_current[3];
> +	u8ÂÂdip_switch;
> +	u8ÂÂhost_interrupt;
> +	u16 voltage_28;
> +	u8ÂÂi2c_device_status;
> +	u8ÂÂpower_status;
> +	u8ÂÂgeneral_status;
> +#define RAVE_SP_STATUS_GS_FIRMWARE_MODE	BIT(1)
> +
> +	u8ÂÂdeprecated1;
> +	u8ÂÂpower_led_status;
> +	u8ÂÂdeprecated2;
> +	u8ÂÂperiph_power_shutoff;
> +} __packed;
> +
> Âstatic bool rave_sp_id_is_event(u8 code)
> Â{
> Â	return (code & 0xF0) == RAVE_SP_EVNT_BASE;
> @@ -609,6 +645,52 @@ static int rave_sp_default_cmd_translate(enum
> rave_sp_command command)
> Â	}
> Â}
> Â
> +static const char *devm_rave_sp_version(struct device *dev,
> +					struct rave_sp_version
> *version)
> +{
> +	/*
> +	Â* NOTE: The format string below uses %02d to display u16
> +	Â* intentionally for the sake of backwards compatibility
> with
> +	Â* legacy software.
> +	Â*/
> +	return devm_kasprintf(dev, GFP_KERNEL,
> "%02d%02d%02d.%c%c\n",
> +			ÂÂÂÂÂÂversion->hardware,
> +			ÂÂÂÂÂÂle16_to_cpu(version->major),
> +			ÂÂÂÂÂÂversion->minor,
> +			ÂÂÂÂÂÂversion->letter[0],
> +			ÂÂÂÂÂÂversion->letter[1]);
> +}
> +
> +static int rave_sp_get_status(struct rave_sp *sp)
> +{
> +	struct device *dev = &sp->serdev->dev;
> +	u8 cmd[] = {
> +		[0] = RAVE_SP_CMD_STATUS,
> +		[1] = 0
> +	};
> +	struct rave_sp_status status;
> +	const char *version;
> +	int ret;
> +
> +	ret = rave_sp_exec(sp, cmd, sizeof(cmd), &status,
> sizeof(status));
> +	if (ret)
> +		return ret;
> +
> +	version = devm_rave_sp_version(dev,
> &status.firmware_version);
> +	if (!version)
> +		return -ENOMEM;
> +
> +	sp->part_number_firmware = version;
> +
> +	version = devm_rave_sp_version(dev,
> &status.bootloader_version);
> +	if (!version)
> +		return -ENOMEM;
> +
> +	sp->part_number_bootloader = version;
> +
> +	return 0;
> +}
> +
> Âstatic const struct rave_sp_checksum rave_sp_checksum_8b2c = {
> Â	.lengthÂÂÂÂÂ= 1,
> Â	.subroutine = csum_8b2c,
> @@ -657,6 +739,7 @@ static const struct serdev_device_ops
> rave_sp_serdev_device_ops = {
> Âstatic int rave_sp_probe(struct serdev_device *serdev)
> Â{
> Â	struct device *dev = &serdev->dev;
> +	const char *unknown = "unknown\n";
> Â	struct rave_sp *sp;
> Â	u32 baud;
> Â	int ret;
> @@ -689,6 +772,20 @@ static int rave_sp_probe(struct serdev_device
> *serdev)
> Â
> Â	serdev_device_set_baudrate(serdev, baud);
> Â
> +	ret = rave_sp_get_status(sp);
> +	if (ret) {
> +		dev_warn(dev, "Failed to get firmware status: %d\n",
> ret);
> +		sp->part_number_firmwareÂÂÂ= unknown;
> +		sp->part_number_bootloader = unknown;
> +	}
> +
> +	/*
> +	Â* Those strings already have a \n embedded, so there's no
> +	Â* need to have one in format string.
> +	Â*/
> +	dev_info(dev, "Firmware version: %s",ÂÂÂsp-
> >part_number_firmware);
> +	dev_info(dev, "Bootloader version: %s", sp-
> >part_number_bootloader);
> +
> Â	return devm_of_platform_populate(dev);
> Â}
> Â