Message-ID: <52d2f247a3058b96625bcdf5ab370282dfc9fb0e.camel@mediatek.com>
Date: Tue, 25 Mar 2025 02:41:26 +0000
From: Axe Yang (杨磊) <Axe.Yang@...iatek.com>
To: Wenbin Mei (梅文彬) <Wenbin.Mei@...iatek.com>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>,
	Chaotian Jing (井朝天)
	<Chaotian.Jing@...iatek.com>, "robh@...nel.org" <robh@...nel.org>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>, "ulf.hansson@...aro.org"
	<ulf.hansson@...aro.org>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	Andy-ld Lu (卢东) <Andy-ld.Lu@...iatek.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Yong Mao (毛勇) <yong.mao@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
	Qingliang Li (黎晴亮) <Qingliang.Li@...iatek.com>
Subject: Re: [PATCH 1/2] dt-bindings: mmc: mtk-sd: add single burst switch

Hi Angelo,

Any comment on this :D 

Regards,
Axe

On Wed, 2025-03-12 at 14:30 +0800, axe.yang wrote:
> On Tue, 2025-03-11 at 10:47 +0100, AngeloGioacchino Del Regno wrote:
> > External email : Please do not click links or open attachments
> > until
> > you have verified the sender or the content.
> > 
> > 
> > Il 07/03/25 07:59, Axe Yang (杨磊) ha scritto:
> > > On Thu, 2025-03-06 at 10:19 +0100, AngeloGioacchino Del Regno
> > > wrote:
> > > > External email : Please do not click links or open attachments
> > > > until
> > > > you have verified the sender or the content.
> > > > 
> > > > 
> > > > Il 06/03/25 09:48, Axe Yang ha scritto:
> > > > > Add 'mediatek,disable-single-burst' setting. This property
> > > > > can
> > > > > be
> > > > > used to switch bus burst type, from single burst to INCR,
> > > > > which
> > > > > is
> > > > > determined by the bus type within the IP. Some versions of
> > > > > the
> > > > > IP
> > > > > are using AXI bus, thus this switch is necessary as 'single'
> > > > > is
> > > > > not
> > > > > the burst type supported by the bus.
> > > > > 
> > > > > Signed-off-by: Axe Yang <axe.yang@...iatek.com>
> > > > 
> > > > I am mostly sure that this is not something to put in
> > > > devicetree,
> > > > but
> > > > as
> > > > platform data for specific SoC(s), as much as I'm mostly sure
> > > > that
> > > > all of
> > > > the instances of the MSDC IP in one SoC will be *all* using
> > > > either
> > > > single
> > > > or INCR.
> > > 
> > > No, actually MSDC IPs in one SoC are using different versions.
> > > Usually MSDC1 (index from 1) is used as eMMC host, the left hosts
> > > are
> > > used as SD/SDIO hosts. They have similar designs, but there are
> > > still
> > > difference.
> > > 
> > > > 
> > > > So, I think I know the answer but I'll still ask just to be
> > > > extremely
> > > > sure:
> > > > 
> > > > is there any MediaTek SoC that has different IP versions for
> > > > different MSDC
> > > > instances, and that hence require single burst on one instance
> > > > and
> > > > INCR on
> > > > another instance?
> > > 
> > > Yes. Actually every SoC has different IP versions for eMMC and
> > > SD/SDIO
> > > host as I said.
> > > e.g. For MT8168, signel burst bit should be set to 1 for eMMC
> > > Host,
> > > but
> > > 0 for SD/SDIO Host.
> > > 
> > > > 
> > > > And if there is - is there a pattern? Is it always SDIO
> > > > requiring
> > > > INCR or
> > > > always eMMC/SD requiring it?
> > > > 
> > > > 
> > > 
> > > No, there is no pattern. Both eMMC and SD/SDIO hosts need to be
> > > configured base on IP version. There is no binding relationship
> > > between
> > > eMMC/SD/SDIO and the burst type. eMMC burst type might be INCR or
> > > single, same as SD/SDIO.
> > > 
> > 
> > Okay but if there are different IP versions, and AXI/AHB is
> > determined
> > by the IP version, why aren't you parsing the MAIN_VER/ECO_VER
> > registers of
> > the MSDC IP to check whether to use INCR or SINGLE?
> 
> 
> To address your concerns, I had further discussions with the
> designer.
> Their response was that the bus type and IP version are not bound
> together. This contradicts my previous statements, and I apologize
> for
> that.
> According to the designer's feedback, I must say that the single
> burst
> setting is indeed tied to the IC, but the granularity is such that it
> needs to be set individually for each host.
> Given the large number of ICs Mediatek currently has, adding burst
> type
> information for each host to the driver's compatible structure would
> mean adding hundreds(maybe thousands :() of lines to the driver for
> the
> compatible structures for *all previous SoCs* (currently there are
> only
> 13 compatible structures, and they can be reuse by new SoC). This
> approach seems very cumbersome.
> 
> So I still believe that placing this setting in the DTS is a more
> appropriate approach.
> 
> Regards,
> Axe
> 
> > 
> > Cheers,
> > Angelo
> > 
> > > 
> > > Regards,
> > > Axe
> > > 
> > > 
> > > > 
> > > > > ---
> > > > >    Documentation/devicetree/bindings/mmc/mtk-sd.yaml | 8
> > > > > ++++++++
> > > > >    1 file changed, 8 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-
> > > > > sd.yaml
> > > > > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > > > > index 0debccbd6519..6076aff0a689 100644
> > > > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > > > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > > > > @@ -100,6 +100,14 @@ properties:
> > > > >        minimum: 0
> > > > >        maximum: 0xffffffff
> > > > > 
> > > > > +  mediatek,disable-single-burst:
> > > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > > +    description:
> > > > > +      Burst type setting. For some versions of the IP that
> > > > > do
> > > > > not
> > > > > use
> > > > > +      AHB bus, the burst type need to be switched to INCR.
> > > > > +      If present, use INCR burst type.
> > > > > +      If not present, use single burst type.
> > > > > +
> > > > >      mediatek,hs200-cmd-int-delay:
> > > > >        $ref: /schemas/types.yaml#/definitions/uint32
> > > > >        description:
> > > > 
> > > > 
> > > > 
> > 
> > 

Powered by blists - more mailing lists