netdev - Re: [PATCH net-next v1 01/13] net: wwan: tmi: Add PCIe core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a3fc5784b38d30a7d7def20aa503d399d336484.camel@mediatek.com>
Date:   Wed, 11 Jan 2023 13:33:14 +0000
From:   Yanchao Yang (杨彦超) 
        <Yanchao.Yang@...iatek.com>
To:     "ryazanov.s.a@...il.com" <ryazanov.s.a@...il.com>
CC:     Chris Feng (冯保林) 
        <Chris.Feng@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "linuxwwan@...iatek.com" <linuxwwan@...iatek.com>,
        Min Dong (董敏) <min.dong@...iatek.com>,
        Mingliang Xu (徐明亮) 
        <mingliang.xu@...iatek.com>,
        "m.chetan.kumar@...el.com" <m.chetan.kumar@...el.com>,
        Liang Lu (吕亮) <liang.lu@...iatek.com>,
        Haijun Liu (刘海军) 
        <haijun.liu@...iatek.com>,
        Haozhe Chang (常浩哲) 
        <Haozhe.Chang@...iatek.com>,
        Hua Yang (杨华) <Hua.Yang@...iatek.com>,
        "linuxwwan@...el.com" <linuxwwan@...el.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "loic.poulain@...aro.org" <loic.poulain@...aro.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "johannes@...solutions.net" <johannes@...solutions.net>,
        Ting Wang (王挺) <ting.wang@...iatek.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Aiden Wang (王咏麒) 
        <Aiden.Wang@...iatek.com>,
        Felix Chen (陈非) <Felix.Chen@...iatek.com>,
        Lambert Wang (王伟) 
        <Lambert.Wang@...iatek.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Mingchuang Qiao (乔明闯) 
        <Mingchuang.Qiao@...iatek.com>,
        Guohao Zhang (张国豪) 
        <Guohao.Zhang@...iatek.com>,
        Xiayu Zhang (张夏宇) 
        <Xiayu.Zhang@...iatek.com>
Subject: Re: [PATCH net-next v1 01/13] net: wwan: tmi: Add PCIe core

Hello Sergey,
sorry for the late response, please check following reply.

On Sat, 2022-12-17 at 00:40 +0400, Sergey Ryazanov wrote:
> Hello Yanchao,
> 
> On 07.12.2022 06:33, Yanchao Yang (杨彦超) wrote:
> > On Sun, 2022-12-04 at 22:52 +0400, Sergey Ryazanov wrote:
> > > On 22.11.2022 15:11, Yanchao Yang wrote:
> > > > Registers the TMI device driver with the kernel. Set up all the
> > > > fundamental
> > > > configurations for the device: PCIe layer, Modem Host Cross
> > > > Core
> > > > Interface
> > > > (MHCCIF), Reset Generation Unit (RGU), modem common control
> > > > operations and
> > > > build infrastructure.
> > > > 
> > > > * PCIe layer code implements driver probe and removal, MSI-X
> > > > interrupt
> > > > initialization and de-initialization, and the way of resetting
> > > > the
> > > > device.
> > > > * MHCCIF provides interrupt channels to communicate events such
> > > > as
> > > > handshake,
> > > > PM and port enumeration.
> > > > * RGU provides interrupt channels to generate notifications
> > > > from
> > > > the device
> > > > so that the TMI driver could get the device reset.
> > > > * Modem common control operations provide the basic read/write
> > > > functions of
> > > > the device's hardware registers, mask/unmask/get/clear
> > > > functions of
> > > > the
> > > > device's interrupt registers and inquiry functions of the
> > > > device's
> > > > status.
> > > > 
> > > > Signed-off-by: Ting Wang <ting.wang@...iatek.com>
> > > > Signed-off-by: MediaTek Corporation <linuxwwan@...iatek.com>
> > > > ---
> > > >    drivers/net/wwan/Kconfig                 |   11 +
> > > >    drivers/net/wwan/Makefile                |    1 +
> > > >    drivers/net/wwan/mediatek/Makefile       |   12 +
> > > >    drivers/net/wwan/mediatek/mtk_common.h   |   30 +
> > > >    drivers/net/wwan/mediatek/mtk_dev.c      |   50 +
> > > >    drivers/net/wwan/mediatek/mtk_dev.h      |  503 ++++++++++
> > > >    drivers/net/wwan/mediatek/pcie/mtk_pci.c | 1164
> > > > ++++++++++++++++++++++
> > > >    drivers/net/wwan/mediatek/pcie/mtk_pci.h |  150 +++
> > > >    drivers/net/wwan/mediatek/pcie/mtk_reg.h |   69 ++
> > > >    9 files changed, 1990 insertions(+)
> > > >    create mode 100644 drivers/net/wwan/mediatek/Makefile
> > > >    create mode 100644 drivers/net/wwan/mediatek/mtk_common.h
> > > >    create mode 100644 drivers/net/wwan/mediatek/mtk_dev.c
> > > >    create mode 100644 drivers/net/wwan/mediatek/mtk_dev.h
> > > >    create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.c
> > > >    create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > > >    create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_reg.h
> > > > 
> > > > diff --git a/drivers/net/wwan/Kconfig
> > > > b/drivers/net/wwan/Kconfig
> > > > index 3486ffe94ac4..a93a0c511d50 100644
> > > > --- a/drivers/net/wwan/Kconfig
> > > > +++ b/drivers/net/wwan/Kconfig
> > > > @@ -119,6 +119,17 @@ config MTK_T7XX
> > > >    
> > > >    	  If unsure, say N.
> > > >    
> > > > +config MTK_TMI
> > > > +	tristate "TMI Driver for Mediatek T-series Device"
> > > > +	depends on PCI
> > > > +	help
> > > > +	  This driver enables Mediatek T-series WWAN Device
> > > > communication.
> > > > +
> > > > +	  If you have one of those Mediatek T-series WWAN
> > > > Modules and
> > > > wish to
> > > > +	  use it in Linux say Y/M here.
> > > 
> > > From this and the series descriptions, it is unclear which modem
> > > chips this driver is intended for and how does it correlate with
> > > the
> > > T7xx driver? Is the TMI driver a drop-in replacement for the t7xx
> > > driver,
> > > or does the TMI driver support any T-series chips except t7xx?
> > > The driver is intended for t8xx or later T-series modem chips in
> > > the
> > 
> > future. Currently, t7xx is not support.
> 
> Can you add this information to the option description to make it
> easier 
> for users to choose?
> 
> BTW, just curious, do you have any plans to add T7xx support to the
> TMI 
> driver, or maybe merge them or factor out the common code into a
> common 
> library? I am asking because I noticed some common code and modem 
> components, but that is not addressed in the cover letter. Or is
> this 
> feeling misleading and these two series are very different?
> 
Sure, next version (v2), we will add description, "TMI is intended for
t8xx T-series modem chips. Currently, t7xx is not supported", to make
option selection more clearly. Yes, we have a plan to support t7xx
after TMI driver is approved and merged. Discussed with t7xx¡¯s
maintainer (Intel), MTK will submit a new patch to support t7xx based
on TMI driver. 
BTW, we receive many comments from patch 1 and patch 2. How about other
patches (patch 3~patch 13)? Do you have any suggestion that we should
wait feedback on other patches or start to submit next version?

many thanks.
yanchao.yang
> > > > +
> > > > +	  If unsure, say N.
> > > > +
> > > >    endif # WWAN
> > > >    
> > > >    endmenu
> > > > diff --git a/drivers/net/wwan/Makefile
> > > > b/drivers/net/wwan/Makefile
> > > > index 3960c0ae2445..198d8074851f 100644
> > > > --- a/drivers/net/wwan/Makefile
> > > > +++ b/drivers/net/wwan/Makefile
> > > > @@ -14,3 +14,4 @@ obj-$(CONFIG_QCOM_BAM_DMUX) +=
> > > > qcom_bam_dmux.o
> > > >    obj-$(CONFIG_RPMSG_WWAN_CTRL) += rpmsg_wwan_ctrl.o
> > > >    obj-$(CONFIG_IOSM) += iosm/
> > > >    obj-$(CONFIG_MTK_T7XX) += t7xx/
> > > > +obj-$(CONFIG_MTK_TMI) += mediatek/
> > > 
> > > The driver is called mtk_tmi, but its code is placed to the
> > > directory
> > > with too generic name 'mediatek'. Do you plan too keep all
> > > possible
> > > future drivers in this directory? >
> > 
> > Yes, we plan to put all mediatek's wwan driver into the same
> > directory.
> > Currently, there is only T-series modem driver. So we don't create
> > 'tmi' folder under 'mediatek' directory explicitly.
> 
> Thank you for the clarification.
> 
> --
> Sergey