linux-kernel - RE: [PATCH v1 1/2] extcon: add Realtek DHC RTD SoC Type-C driver

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] [day] [month] [year] [list]

Message-ID: <249af57804f3405f80f28879e31637b3@realtek.com>
Date:   Wed, 30 Aug 2023 03:59:31 +0000
From:   Stanley Chang[昌育德] 
        <stanley_chang@...ltek.com>
To:     Chanwoo Choi <chanwoo@...nel.org>,
        MyungJoo Ham <myungjoo.ham@...sung.com>
CC:     Chanwoo Choi <cw00.choi@...sung.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [PATCH v1 1/2] extcon: add Realtek DHC RTD SoC Type-C driver

Hi Chanwoo,

> >>> +#define TYPE_C_EN_SWITCH BIT(29)
> >>> +#define TYPE_C_TXRX_SEL (BIT(28) | BIT(27)) #define
> >>> +TYPE_C_SWITCH_MASK (TYPE_C_EN_SWITCH | TYPE_C_TXRX_SEL)
> #define
> >>> +TYPE_C_ENABLE_CC1 TYPE_C_EN_SWITCH #define
> TYPE_C_ENABLE_CC2
> >>> +(TYPE_C_EN_SWITCH | TYPE_C_TXRX_SEL) #define TYPE_C_DISABLE_CC
> >>> +~TYPE_C_SWITCH_MASK
> >>> +
> >>> +     val_cc = readl(reg);
> >>
> >> I'd like you to use regmap interface to access the register by using
> >> regmap_read, regmap_write. You can create the regmap instance via
> >> devm_regmap_init_mmio() on probe instead of using 'type_c->reg_base'
> >> at the multipe point.
> >>
> >> For example,
> >>         struct regmap_config rtk_regmap_config = {
> >>                 .reg_bits = 32,
> >>                 .val_bits = 32,
> >>         };
> >>
> >>         void __iomem *base;
> >>
> >>         base = devm_platform_get_and_ioremap_resource(pdev, 0,
> &res);
> >>         if (IS_ERR(base))
> >>                 return PTR_ERR(base);
> >>
> >>         regmap = devm_regmap_init_mmio(dev, base,
> >> &rtk_regmap_config);
> >>
> >>         ---
> >>
> >>         And then just call regmap_read without any calculation between
> >>         base address and specific register.
> >>
> >>         regmap_read(regmap, USB_TYPEC_CTRL_CC1_0)
> >>
> >
> > I studied mmio's regmap.
> >
> > It only changed one encoding method. And simplifies the calculation
> between the base address and the specific register.
> 
> The regmap provides the consistent interface to access register regardless of
> mmio/i2c/spi and so on. It is the advantage of regmap.
> 
> And regmap provides the like register dump via debugfs, is able to specify the
> kind of register like writable, readable, volatile type.
> It is possible to specify the more detailed register information in device driver
> in order to improve the readability. It allows you to express more detail of this
> device than just working.

These are all advantages of regmap, I agree.
I haven't delved into regmap yet.

> 
> > If the register is 32-bit aligned, other operations look the same as
> readl/writel.
> > I think regmap is more simplified if the read registers are not 32-bit aligned,
> e.g. nvmem read/write.
> 
> I'm sorry. I don't understand of what is accurate meaning.
> Could you please explain your opinion more detailed with example of this
> patch?

Sorry, I misunderstood regmap, I thought he could read register like read otp tables by nvmem regardless of his bitness and size.
So my answer should be incorrect.

Thank,
Stanley