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: <02dabcc8-2340-2188-f6a8-51513b147e7c@quicinc.com>
Date:   Tue, 13 Jun 2023 21:36:41 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH] soc: qcom: geni-se: Do not bother about enable/disable of
 interrupts in secondary sequencer for non-uart modes
Content-Language: en-CA
From:   Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
To:     Doug Anderson <dianders@chromium.org>
CC:     <agross@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_msavaliy@quicinc.com>,
        <mka@chromium.org>, <swboyd@chromium.org>,
        <quic_vtanuku@quicinc.com>
References: <1685729609-26871-1-git-send-email-quic_vnivarth@quicinc.com>
 <CAD=FV=Uy=ELwg2jhtFSrpndw-GxUO=0NTKotNymi3sqwG=ggnQ@mail.gmail.com>
 <af4c131a-b97d-a8e8-957d-77c31d3c816a@quicinc.com>
In-Reply-To: <af4c131a-b97d-a8e8-957d-77c31d3c816a@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,


On 6/12/2023 7:09 PM, Vijaya Krishna Nivarthi wrote:
> Hi,
>
> Thank you very much for the review...
>
>
> On 6/7/2023 9:41 PM, Doug Anderson wrote:
>> Hi,
>>
>> On Fri, Jun 2, 2023 at 11:13 AM Vijaya Krishna Nivarthi
>> <quic_vnivarth@quicinc.com> wrote:
>>> The select_fifo/dma_mode() functions in geni driver enable/disable
>>> interrupts (secondary included) conditionally for non-uart modes, while
>>> uart is supposed to manage this internally.
>>> However, only uart uses secondary IRQs while spi, i2c do not care about
>>> these at all making their enablement (or disablement) totally 
>>> unnecessary
>>> for these protos.
>>>
>>> Drop enabling/disabling secondary IRQs for non-uart modes.
>>> This doesn't solve any observed problem but only gets rid of code 
>>> pieces
>>> that are not required.
>>>
>>> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
>>> ---
>>>   drivers/soc/qcom/qcom-geni-se.c | 24 ++++--------------------
>>>   1 file changed, 4 insertions(+), 20 deletions(-)
>> This seems like a nice cleanup to me. It's still odd that the general
>> code has a special case for UART, but I guess the alternative is to
>> duplicate the exact same logic for both the i2c and SPI drivers. I
>> won't insist on that, though I wouldn't be opposed to it either.
>>
>> I guess one comment, though: should we also remove the code that
>> messes with "SE_GENI_S_IRQ_EN" in geni_se_select_gpi_mode()?
>
>
> Right now we have gpi-dma mode support for i2c and spi but not for uart.
>
> Even when gpi-dma support is added, uart driver's interrupt handler 
> would not be invoked so I believe it would be safe to drop that code 
> from geni_se_select_gpi_mode() too.
>
> I will post v2 dropping same after some more lookup.


Looking at this once again, I am more inclined towards leaving alone 
gpi_mode() for now.

It probably needs cleanup but we want to take that up at a later time.

Meanwhile its not causing much harm as we don't switch modes dynamically 
for gpi case, so we are not losing much time there reading from and 
writing to registers.

Please let know your thoughts.

Thank you,

Vijay/


>
> -Vijay/
>
>
>>
>> -Doug