Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211101035635.26999-1-ricardo.martinez@linux.intel.com> <20211101035635.26999-7-ricardo.martinez@linux.intel.com>
In-Reply-To: <20211101035635.26999-7-ricardo.martinez@linux.intel.com>
From:   Sergey Ryazanov <ryazanov.s.a@gmail.com>
Date:   Tue, 9 Nov 2021 15:06:26 +0300
Message-ID: <CAHNKnsRe-88_jXvW4=0rPSDhVTbnJnDoeLpjHS4ouDv3pJXWSg@mail.gmail.com>
Subject: Re: [PATCH v2 06/14] net: wwan: t7xx: Add AT and MBIM WWAN ports
To:     Ricardo Martinez <ricardo.martinez@linux.intel.com>
Cc:     netdev@vger.kernel.org, linux-wireless@vger.kernel.org,
        Jakub Kicinski <kuba@kernel.org>,
        David Miller <davem@davemloft.net>,
        Johannes Berg <johannes@sipsolutions.net>,
        Loic Poulain <loic.poulain@linaro.org>,
        M Chetan Kumar <m.chetan.kumar@intel.com>,
        chandrashekar.devegowda@intel.com,
        Intel Corporation <linuxwwan@intel.com>,
        chiranjeevi.rapolu@linux.intel.com, haijun.liu@mediatek.com,
        amir.hanania@intel.com,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        dinesh.sharma@intel.com, eliot.lee@intel.com,
        mika.westerberg@linux.intel.com, moises.veleta@intel.com,
        pierre-louis.bossart@intel.com, muralidharan.sethuraman@intel.com,
        Soumya.Prakash.Mishra@intel.com, sreehari.kancharla@intel.com,
        suresh.nagaraj@intel.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Nov 1, 2021 at 6:57 AM Ricardo Martinez wrote:
> ...
>  static struct t7xx_port md_ccci_ports[] = {
> +       {CCCI_UART2_TX, CCCI_UART2_RX, DATA_AT_CMD_Q, DATA_AT_CMD_Q, 0xff,
> +        0xff, ID_CLDMA1, PORT_F_RX_CHAR_NODE, &wwan_sub_port_ops, 0, "ttyC0", WWAN_PORT_AT},
> +       {CCCI_MBIM_TX, CCCI_MBIM_RX, 2, 2, 0, 0, ID_CLDMA1,
> +        PORT_F_RX_CHAR_NODE, &wwan_sub_port_ops, 10, "ttyCMBIM0", WWAN_PORT_MBIM},
> ...
> +               if (count + CCCI_H_ELEN > txq_mtu &&
> +                   (port_ccci->tx_ch == CCCI_MBIM_TX ||
> +                    (port_ccci->tx_ch >= CCCI_DSS0_TX && port_ccci->tx_ch <= CCCI_DSS7_TX)))
> +                       multi_packet = DIV_ROUND_UP(count, txq_mtu - CCCI_H_ELEN);

I am just wondering, the chip does support MBIM message fragmentation,
but does not support AT commands stream (CCCI_UART2_TX) fragmentation.
Is that the correct conclusion from the code above?

BTW, you could factor out data fragmentation support to a dedicated
function to improve code readability. Something like this:

static inline bool port_is_multipacket_capable(... *port)
{
        return port->tx_ch == CCCI_MBIM_TX ||
               (port->tx_ch >= CCCI_DSS0_TX && port->tx_ch <= CCCI_DSS7_TX);
}

So condition become something like that:

        if (count + CCCI_H_ELEN > txq_mtu &&
            port_is_multipacket_capable(port))
                multi_packet = DIV_ROUND_UP(count, txq_mtu - CCCI_H_ELEN);

-- 
Sergey