Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941196AbcJSQWo (ORCPT ); Wed, 19 Oct 2016 12:22:44 -0400 Received: from foss.arm.com ([217.140.101.70]:53278 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S938940AbcJSOPD (ORCPT ); Wed, 19 Oct 2016 10:15:03 -0400 Subject: Re: [PATCH v4 2/8] scpi: Add alternative legacy structures, functions and macros To: Neil Armstrong , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <1475652814-30619-1-git-send-email-narmstrong@baylibre.com> <1475652814-30619-3-git-send-email-narmstrong@baylibre.com> <7bf3b68c-19aa-85c3-8b20-66b7380eaf3f@arm.com> <24e6ef19-b88d-a5b8-f514-d39a7b5725e8@baylibre.com> Cc: Sudeep Holla , linux-amlogic@lists.infradead.org, khilman@baylibre.com, heiko@sntech.de, wxt@rock-chips.com, frank.wang@rock-chips.com From: Sudeep Holla Organization: ARM Message-ID: Date: Wed, 19 Oct 2016 11:37:23 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <24e6ef19-b88d-a5b8-f514-d39a7b5725e8@baylibre.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1578 Lines: 59 On 19/10/16 11:28, Neil Armstrong wrote: > On 10/17/2016 01:16 PM, Sudeep Holla wrote: [...] >> >> and the above 2 can be moved out of the conditions, no ? >> >> if (scpi_info->is_legacy) { >> struct legacy_scpi_shared_mem *mem = ch->rx_payload; >> len = match->rx_len; >> } else { >> struct scpi_shared_mem *mem = ch->rx_payload; >> len = min(match->rx_len, CMD_SIZE(cmd)); >> } >> match->status = le32_to_cpu(mem->status); >> memcpy_fromio(match->rx_buf, mem->payload, len); >> >> should work. > > Well, we will have "error: ?mem? undeclared (first use in this > function)" since mem is not declared outside the if/else. > > I don't see good solutions even with an union. Right, I missed to see that. You can leave it as you had before then. [...] >>>>> if (t->rx_buf) { >>>>> if (!(++ch->token)) >>>>> ++ch->token; >>>>> ADD_SCPI_TOKEN(t->cmd, ch->token); >>>>> + if (scpi_info->is_legacy) >>>>> + t->slot = t->cmd; >>>> >>>> I thought passing token was not an issue from your previous response, >>>> but you are overriding it here, why ? >>> >>> Indeed, I can leave it, but it's useless since it won't serve to >>> distinguish multiple similar commands. >>> >> >> OK, I don't see any point in such micro optimization, so please retain it. >> > > I misread my code, I leaved the token passing, but I copy back the > cmd to the slot which is used by the MHU. > If I remove the "t->slot = t->cmd;", the token won't be passed to the > FW. > Right, sorry I think I misled you :) -- Regards, Sudeep