On Tue, 2018-06-26 at 09:59 -0600, Mathieu Poirier wrote:

On Mon, Jun 25, 2018 at 09:23:51AM -0700, Joe Perches wrote:

quoted

On Mon, 2018-06-25 at 12:22 +0100, Suzuki K Poulose wrote:

quoted

Add the initial support for Coresight Address Translation Unit, which
augments the TMC in Coresight SoC-600 by providing an improved Scatter
Gather mechanism. CATU is always connected to a single TMC-ETR and
converts the AXI address with a translated address (from a given SG
table with specific format). The CATU should be programmed in pass
through mode and enabled even if the ETR doesn't use the translation
by CATU.

[]

quoted

diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h

[]

quoted

+static inline bool coresight_is_catu_device(struct coresight_device *csdev)
+{
+	enum coresight_dev_subtype_helper subtype;
+
+	/* Make the checkpatch happy */
+	subtype = csdev->subtype.helper_subtype;
+
+	return IS_ENABLED(CONFIG_CORESIGHT_CATU) &&
+	       csdev->type == CORESIGHT_DEV_TYPE_HELPER &&
+	       subtype == CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
+}

I believe you should not make checkpatch happy here
by using a temporary but use the most readable form.

Probably the most readable form is:

	return IS_ENABLED(CONFIG_CORESIGHT_CATU) &&
	       csdev->type == CORESIGHT_DEV_TYPE_HELPER &&
	       csdev->subtype.helper_subtype == CORESIGHT_DEV_SUBTYPE_HELPER_CATU;

where restricting line length to 80 columns is not useful
because the identifiers are very long.  The identifiers
are possibly longer than necessary.

I would prefer to avoid adding code that will trigger checkpatch warnings.  I
suggest the following:

static inline bool coresight_is_catu_device(struct coresight_device *csdev)
{
        if (!IS_ENABLED(CONFIG_CORESIGHT_CATU))
                return false;

        if (csdev->type != CORESIGHT_DEV_TYPE_HELPER)
                return false;

        if (csdev->subtype.helper_subtype != CORESIGHT_DEV_SUBTYPE_HELPER_CATU)
                return false;

        return true;
}

No temporary variable and all shorther than 80 columns.

Perhaps the most readable uses the positive
form instead of the negative, but your choice.
The compiler should emit equivalent or identical
code anyway.

Do remember that checkpatch is brainless and
everyone should always use theirs over fixing
any message that checkpatch emits.

cheers, Joe