Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3d69be4a-7856-11a3-fddf-0b8afa645827@redhat.com>
Date:   Wed, 22 Jun 2022 12:09:59 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH] platform/surface: aggregator: Reserve more event- and
 target-categories
Content-Language: en-US
To:     Maximilian Luz <luzmaximilian@gmail.com>
Cc:     Mark Gross <markgross@kernel.org>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220614194117.4118897-1-luzmaximilian@gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20220614194117.4118897-1-luzmaximilian@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 6/14/22 21:41, Maximilian Luz wrote:
> With the introduction of the Surface Laptop Studio, more event- and
> target categories have been added. Therefore, increase the number of
> reserved events and extend the enum of know target categories to
> accommodate this.
> 
> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans

> ---
>  drivers/platform/surface/aggregator/trace.h   | 80 +++++++++++--------
>  include/linux/surface_aggregator/serial_hub.h | 75 +++++++++--------
>  2 files changed, 85 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/platform/surface/aggregator/trace.h b/drivers/platform/surface/aggregator/trace.h
> index de64cf169060..cc9e73fbc18e 100644
> --- a/drivers/platform/surface/aggregator/trace.h
> +++ b/drivers/platform/surface/aggregator/trace.h
> @@ -76,7 +76,7 @@ TRACE_DEFINE_ENUM(SSAM_SSH_TC_HID);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_TCH);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_BKL);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_TAM);
> -TRACE_DEFINE_ENUM(SSAM_SSH_TC_ACC);
> +TRACE_DEFINE_ENUM(SSAM_SSH_TC_ACC0);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_UFI);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_USC);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_PEN);
> @@ -85,6 +85,11 @@ TRACE_DEFINE_ENUM(SSAM_SSH_TC_AUD);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_SMC);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_KPD);
>  TRACE_DEFINE_ENUM(SSAM_SSH_TC_REG);
> +TRACE_DEFINE_ENUM(SSAM_SSH_TC_SPT);
> +TRACE_DEFINE_ENUM(SSAM_SSH_TC_SYS);
> +TRACE_DEFINE_ENUM(SSAM_SSH_TC_ACC1);
> +TRACE_DEFINE_ENUM(SSAM_SSH_TC_SHB);
> +TRACE_DEFINE_ENUM(SSAM_SSH_TC_POS);
>  
>  #define SSAM_PTR_UID_LEN		9
>  #define SSAM_U8_FIELD_NOT_APPLICABLE	((u16)-1)
> @@ -229,40 +234,45 @@ static inline u32 ssam_trace_get_request_tc(const struct ssh_packet *p)
>  
>  #define ssam_show_ssh_tc(rqid)						\
>  	__print_symbolic(rqid,						\
> -		{ SSAM_SSH_TC_NOT_APPLICABLE,		"N/A" },	\
> -		{ SSAM_SSH_TC_SAM,			"SAM" },	\
> -		{ SSAM_SSH_TC_BAT,			"BAT" },	\
> -		{ SSAM_SSH_TC_TMP,			"TMP" },	\
> -		{ SSAM_SSH_TC_PMC,			"PMC" },	\
> -		{ SSAM_SSH_TC_FAN,			"FAN" },	\
> -		{ SSAM_SSH_TC_PoM,			"PoM" },	\
> -		{ SSAM_SSH_TC_DBG,			"DBG" },	\
> -		{ SSAM_SSH_TC_KBD,			"KBD" },	\
> -		{ SSAM_SSH_TC_FWU,			"FWU" },	\
> -		{ SSAM_SSH_TC_UNI,			"UNI" },	\
> -		{ SSAM_SSH_TC_LPC,			"LPC" },	\
> -		{ SSAM_SSH_TC_TCL,			"TCL" },	\
> -		{ SSAM_SSH_TC_SFL,			"SFL" },	\
> -		{ SSAM_SSH_TC_KIP,			"KIP" },	\
> -		{ SSAM_SSH_TC_EXT,			"EXT" },	\
> -		{ SSAM_SSH_TC_BLD,			"BLD" },	\
> -		{ SSAM_SSH_TC_BAS,			"BAS" },	\
> -		{ SSAM_SSH_TC_SEN,			"SEN" },	\
> -		{ SSAM_SSH_TC_SRQ,			"SRQ" },	\
> -		{ SSAM_SSH_TC_MCU,			"MCU" },	\
> -		{ SSAM_SSH_TC_HID,			"HID" },	\
> -		{ SSAM_SSH_TC_TCH,			"TCH" },	\
> -		{ SSAM_SSH_TC_BKL,			"BKL" },	\
> -		{ SSAM_SSH_TC_TAM,			"TAM" },	\
> -		{ SSAM_SSH_TC_ACC,			"ACC" },	\
> -		{ SSAM_SSH_TC_UFI,			"UFI" },	\
> -		{ SSAM_SSH_TC_USC,			"USC" },	\
> -		{ SSAM_SSH_TC_PEN,			"PEN" },	\
> -		{ SSAM_SSH_TC_VID,			"VID" },	\
> -		{ SSAM_SSH_TC_AUD,			"AUD" },	\
> -		{ SSAM_SSH_TC_SMC,			"SMC" },	\
> -		{ SSAM_SSH_TC_KPD,			"KPD" },	\
> -		{ SSAM_SSH_TC_REG,			"REG" }		\
> +		{ SSAM_SSH_TC_NOT_APPLICABLE,		"N/A"  },	\
> +		{ SSAM_SSH_TC_SAM,			"SAM"  },	\
> +		{ SSAM_SSH_TC_BAT,			"BAT"  },	\
> +		{ SSAM_SSH_TC_TMP,			"TMP"  },	\
> +		{ SSAM_SSH_TC_PMC,			"PMC"  },	\
> +		{ SSAM_SSH_TC_FAN,			"FAN"  },	\
> +		{ SSAM_SSH_TC_PoM,			"PoM"  },	\
> +		{ SSAM_SSH_TC_DBG,			"DBG"  },	\
> +		{ SSAM_SSH_TC_KBD,			"KBD"  },	\
> +		{ SSAM_SSH_TC_FWU,			"FWU"  },	\
> +		{ SSAM_SSH_TC_UNI,			"UNI"  },	\
> +		{ SSAM_SSH_TC_LPC,			"LPC"  },	\
> +		{ SSAM_SSH_TC_TCL,			"TCL"  },	\
> +		{ SSAM_SSH_TC_SFL,			"SFL"  },	\
> +		{ SSAM_SSH_TC_KIP,			"KIP"  },	\
> +		{ SSAM_SSH_TC_EXT,			"EXT"  },	\
> +		{ SSAM_SSH_TC_BLD,			"BLD"  },	\
> +		{ SSAM_SSH_TC_BAS,			"BAS"  },	\
> +		{ SSAM_SSH_TC_SEN,			"SEN"  },	\
> +		{ SSAM_SSH_TC_SRQ,			"SRQ"  },	\
> +		{ SSAM_SSH_TC_MCU,			"MCU"  },	\
> +		{ SSAM_SSH_TC_HID,			"HID"  },	\
> +		{ SSAM_SSH_TC_TCH,			"TCH"  },	\
> +		{ SSAM_SSH_TC_BKL,			"BKL"  },	\
> +		{ SSAM_SSH_TC_TAM,			"TAM"  },	\
> +		{ SSAM_SSH_TC_ACC0,			"ACC0" },	\
> +		{ SSAM_SSH_TC_UFI,			"UFI"  },	\
> +		{ SSAM_SSH_TC_USC,			"USC"  },	\
> +		{ SSAM_SSH_TC_PEN,			"PEN"  },	\
> +		{ SSAM_SSH_TC_VID,			"VID"  },	\
> +		{ SSAM_SSH_TC_AUD,			"AUD"  },	\
> +		{ SSAM_SSH_TC_SMC,			"SMC"  },	\
> +		{ SSAM_SSH_TC_KPD,			"KPD"  },	\
> +		{ SSAM_SSH_TC_REG,			"REG"  },	\
> +		{ SSAM_SSH_TC_SPT,			"SPT"  },	\
> +		{ SSAM_SSH_TC_SYS,			"SYS"  },	\
> +		{ SSAM_SSH_TC_ACC1,			"ACC1" },	\
> +		{ SSAM_SSH_TC_SHB,			"SMB"  },	\
> +		{ SSAM_SSH_TC_POS,			"POS"  }	\
>  	)
>  
>  DECLARE_EVENT_CLASS(ssam_frame_class,
> diff --git a/include/linux/surface_aggregator/serial_hub.h b/include/linux/surface_aggregator/serial_hub.h
> index 26b95ec12733..45501b6e54e8 100644
> --- a/include/linux/surface_aggregator/serial_hub.h
> +++ b/include/linux/surface_aggregator/serial_hub.h
> @@ -201,7 +201,7 @@ static inline u16 ssh_crc(const u8 *buf, size_t len)
>   * exception of zero, which is not an event ID. Thus, this is also the
>   * absolute maximum number of event handlers that can be registered.
>   */
> -#define SSH_NUM_EVENTS		34
> +#define SSH_NUM_EVENTS		38
>  
>  /*
>   * SSH_NUM_TARGETS - The number of communication targets used in the protocol.
> @@ -292,40 +292,45 @@ struct ssam_span {
>   * Windows driver.
>   */
>  enum ssam_ssh_tc {
> -				/* Category 0x00 is invalid for EC use. */
> -	SSAM_SSH_TC_SAM = 0x01,	/* Generic system functionality, real-time clock. */
> -	SSAM_SSH_TC_BAT = 0x02,	/* Battery/power subsystem. */
> -	SSAM_SSH_TC_TMP = 0x03,	/* Thermal subsystem. */
> -	SSAM_SSH_TC_PMC = 0x04,
> -	SSAM_SSH_TC_FAN = 0x05,
> -	SSAM_SSH_TC_PoM = 0x06,
> -	SSAM_SSH_TC_DBG = 0x07,
> -	SSAM_SSH_TC_KBD = 0x08,	/* Legacy keyboard (Laptop 1/2). */
> -	SSAM_SSH_TC_FWU = 0x09,
> -	SSAM_SSH_TC_UNI = 0x0a,
> -	SSAM_SSH_TC_LPC = 0x0b,
> -	SSAM_SSH_TC_TCL = 0x0c,
> -	SSAM_SSH_TC_SFL = 0x0d,
> -	SSAM_SSH_TC_KIP = 0x0e,	/* Manages detachable peripherals (Pro X/8 keyboard cover) */
> -	SSAM_SSH_TC_EXT = 0x0f,
> -	SSAM_SSH_TC_BLD = 0x10,
> -	SSAM_SSH_TC_BAS = 0x11,	/* Detachment system (Surface Book 2/3). */
> -	SSAM_SSH_TC_SEN = 0x12,
> -	SSAM_SSH_TC_SRQ = 0x13,
> -	SSAM_SSH_TC_MCU = 0x14,
> -	SSAM_SSH_TC_HID = 0x15,	/* Generic HID input subsystem. */
> -	SSAM_SSH_TC_TCH = 0x16,
> -	SSAM_SSH_TC_BKL = 0x17,
> -	SSAM_SSH_TC_TAM = 0x18,
> -	SSAM_SSH_TC_ACC = 0x19,
> -	SSAM_SSH_TC_UFI = 0x1a,
> -	SSAM_SSH_TC_USC = 0x1b,
> -	SSAM_SSH_TC_PEN = 0x1c,
> -	SSAM_SSH_TC_VID = 0x1d,
> -	SSAM_SSH_TC_AUD = 0x1e,
> -	SSAM_SSH_TC_SMC = 0x1f,
> -	SSAM_SSH_TC_KPD = 0x20,
> -	SSAM_SSH_TC_REG = 0x21,	/* Extended event registry. */
> +				  /* Category 0x00 is invalid for EC use. */
> +	SSAM_SSH_TC_SAM  = 0x01,  /* Generic system functionality, real-time clock. */
> +	SSAM_SSH_TC_BAT  = 0x02,  /* Battery/power subsystem. */
> +	SSAM_SSH_TC_TMP  = 0x03,  /* Thermal subsystem. */
> +	SSAM_SSH_TC_PMC  = 0x04,
> +	SSAM_SSH_TC_FAN  = 0x05,
> +	SSAM_SSH_TC_PoM  = 0x06,
> +	SSAM_SSH_TC_DBG  = 0x07,
> +	SSAM_SSH_TC_KBD  = 0x08,  /* Legacy keyboard (Laptop 1/2). */
> +	SSAM_SSH_TC_FWU  = 0x09,
> +	SSAM_SSH_TC_UNI  = 0x0a,
> +	SSAM_SSH_TC_LPC  = 0x0b,
> +	SSAM_SSH_TC_TCL  = 0x0c,
> +	SSAM_SSH_TC_SFL  = 0x0d,
> +	SSAM_SSH_TC_KIP  = 0x0e,  /* Manages detachable peripherals (Pro X/8 keyboard cover) */
> +	SSAM_SSH_TC_EXT  = 0x0f,
> +	SSAM_SSH_TC_BLD  = 0x10,
> +	SSAM_SSH_TC_BAS  = 0x11,  /* Detachment system (Surface Book 2/3). */
> +	SSAM_SSH_TC_SEN  = 0x12,
> +	SSAM_SSH_TC_SRQ  = 0x13,
> +	SSAM_SSH_TC_MCU  = 0x14,
> +	SSAM_SSH_TC_HID  = 0x15,  /* Generic HID input subsystem. */
> +	SSAM_SSH_TC_TCH  = 0x16,
> +	SSAM_SSH_TC_BKL  = 0x17,
> +	SSAM_SSH_TC_TAM  = 0x18,
> +	SSAM_SSH_TC_ACC0 = 0x19,
> +	SSAM_SSH_TC_UFI  = 0x1a,
> +	SSAM_SSH_TC_USC  = 0x1b,
> +	SSAM_SSH_TC_PEN  = 0x1c,
> +	SSAM_SSH_TC_VID  = 0x1d,
> +	SSAM_SSH_TC_AUD  = 0x1e,
> +	SSAM_SSH_TC_SMC  = 0x1f,
> +	SSAM_SSH_TC_KPD  = 0x20,
> +	SSAM_SSH_TC_REG  = 0x21,  /* Extended event registry. */
> +	SSAM_SSH_TC_SPT  = 0x22,
> +	SSAM_SSH_TC_SYS  = 0x23,
> +	SSAM_SSH_TC_ACC1 = 0x24,
> +	SSAM_SSH_TC_SHB  = 0x25,
> +	SSAM_SSH_TC_POS  = 0x26,  /* For obtaining Laptop Studio screen position. */
>  };
>  
>