Re: [PATCH v1 07/11] intel_th: Add Global Trace Hub driver

[Joe Perches <joe@perches.com>]

On Fri, 2015-03-20 at 19:29 +0200, Alexander Shishkin wrote:
> Global Trace Hub (GTH) is the central component of Intel TH architecture;
> it carries out switching between the trace sources and trace outputs, can
> enable/disable tracing, perform STP encoding, internal buffering, control
> backpressure from outputs to sources and so on.

trivia:

> diff --git a/drivers/hwtracing/intel_th/gth.c b/drivers/hwtracing/intel_th/gth.c

> +struct gth_output {
> +	struct gth_device	*gth;
> +	struct intel_th_output	*output;
> +	unsigned int		index;
> +	unsigned int		port_type;
> +	unsigned long		master[TH_CONFIGURABLE_MASTERS + 1 /
> +				       BITS_PER_LONG];

	DECLARE_BITMAP(master, TH_CONFIGURABLE_MASTERS+ 1); ?

> +};
> +
> +/**
> + * struct gth_device - GTH device
> + * @dev:	driver core's device
> + * @base:	register window base address
> + * @output_group:	attributes describing output ports
> + * @master_group:	attributes describing master assignments
> + * @output:		output ports
> + * @master:		master/output port assignments
> + * @gth_lock:		serializes accesses to GTH bits
> + */
> +struct gth_device {
> +	struct device		*dev;
> +	void __iomem		*base;
> +
> +	struct attribute_group	output_group;
> +	struct attribute_group	master_group;
> +	struct gth_output	output[TH_POSSIBLE_OUTPUTS];
> +	signed char		master[TH_CONFIGURABLE_MASTERS + 1];

why signed?

[]

> +#define OUTPUT_PARM(_name, _mask, _r, _w)				\
> +	[TH_OUTPUT_PARM(_name)] = { .name = __stringify(_name),		\
> +				    .mask = (_mask),			\
> +				    .readable = (_r),			\
> +				    .writable = (_w) }
> +
> +static struct output_parm {

const

> +	const char	*name;
> +	unsigned int	mask;
> +	unsigned int	readable : 1,
> +			writable : 1;
> +} output_parms[] = {
> +	OUTPUT_PARM(port,	0x7,	1, 0),
> +	OUTPUT_PARM(null,	BIT(3),	1, 1),
> +	OUTPUT_PARM(drop,	BIT(4),	1, 1),
> +	OUTPUT_PARM(reset,	BIT(5),	1, 0),
> +	OUTPUT_PARM(flush,	BIT(7),	0, 1),
> +};

[]

> +static int intel_th_master_attributes(struct gth_device *gth)
> +{
> +	struct master_attribute *master_attrs;
> +	struct attribute **attrs;
> +	int i, nattrs = TH_CONFIGURABLE_MASTERS + 2;
> +
> +	attrs = devm_kzalloc(gth->dev, sizeof(void *) * nattrs, GFP_KERNEL);
> +	if (!attrs)
> +		return -ENOMEM;
> +
> +	master_attrs = devm_kzalloc(gth->dev,
> +				    sizeof(struct master_attribute) * nattrs,
> +				    GFP_KERNEL);

devm_kcalloc