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

[Yanchao Yang (杨彦超)]

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@xxxxxxxxxxxx>
> > > > Signed-off-by: MediaTek Corporation <linuxwwan@xxxxxxxxxxxx>
> > > > ---
> > > >    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