From: Lee Jones <lee.jones@linaro.org>
To: Gene Chen <gene.chen.richtek@gmail.com>
Cc: Gene Chen <gene_chen@richtek.com>,
rafael@kernel.org, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, cy_huang@richtek.com,
benjamin.chao@mediatek.com, Mark Brown <broonie@kernel.org>,
linux-mediatek@lists.infradead.org,
Matthias Brugger <matthias.bgg@gmail.com>,
linux-arm-kernel@lists.infradead.org, shufan_lee@richtek.com
Subject: Re: [PATCH 9/9] mfd: mt6360: Merge different sub-devices I2C read/write
Date: Mon, 10 Aug 2020 08:08:43 +0100 [thread overview]
Message-ID: <20200810070843.GA4411@dell> (raw)
In-Reply-To: <CAE+NS37tr65GnTue89wJkPvJzddahKj__KPgzmjzwkOfWQnc8g@mail.gmail.com>
On Fri, 07 Aug 2020, Gene Chen wrote:
> Mark Brown <broonie@kernel.org> 於 2020年8月6日 週四 下午8:13寫道:
> >
> > On Thu, Aug 06, 2020 at 11:30:56AM +0800, Gene Chen wrote:
> > > Mark Brown <broonie@kernel.org> 於 2020年8月6日 週四 上午12:10寫道:
> >
> > > > It's not clear why this isn't just done in the device regmap, there's
> > > > exactly one user?
> >
> > > because I use one regmap to access 4 I2C devices,
> >
> > There appears to be only one device here?
> >
> > > I need change the regmap_bus struct to fit I2C read/write with CRC bit
> > > Therefore, MFD reviewer suggests I can move the regmap api to regmap
> > > folder such as regmap-ac97.c
> >
> > AC'97 is an industry standard bus used by a range of devices in
> > different subsystems. You can already have custom operations for a
> > device just in a regular regmap using the reg_read() and reg_write()
> > operations which are there so devices that individual device support
> > doesn't need to be added to the regmap core.
> >
>
> I need use regmap_raw_read to access MT6360 TYPEC part, so we need
> implement bus read control
>
> > You really also need to write a much clearer changelog, I would be hard
> > pressed to tell from the changelog that this was moving things to the
> > regmap core rather than shuffling regmaps within the device.
>
> MT6360 has 4 I2C worker devices
> First, I increase reg_bits from 8 to 16 bits.
> Higher 8 bits, bank, indicated which worker device I want access
> Then, if worker devices is PMIC or LDO part, I need calculate or check
> CRC8 bits when we write or read data.
> CRC8 bits is calculated by 3 parts.
> 1'st part include 1 byte is worker address and R/W in LSB.
> 2'nd part include 1 byte is register address
> 3'nd part include written data or read data from MT6360
> I also need 1 dummy byte when write data
>
> @Lee Jones,
> I found out drivers/iio/chemical/bme680_spi.c implement their own
> regmap_bus too.
> Can I move regmap control back to mt6360-core.c?
Yes, if that is the 'done thing'.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: Gene Chen <gene.chen.richtek@gmail.com>
Cc: Gene Chen <gene_chen@richtek.com>,
rafael@kernel.org, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, cy_huang@richtek.com,
benjamin.chao@mediatek.com, Mark Brown <broonie@kernel.org>,
linux-mediatek@lists.infradead.org,
Matthias Brugger <matthias.bgg@gmail.com>,
linux-arm-kernel@lists.infradead.org, shufan_lee@richtek.com
Subject: Re: [PATCH 9/9] mfd: mt6360: Merge different sub-devices I2C read/write
Date: Mon, 10 Aug 2020 08:08:43 +0100 [thread overview]
Message-ID: <20200810070843.GA4411@dell> (raw)
In-Reply-To: <CAE+NS37tr65GnTue89wJkPvJzddahKj__KPgzmjzwkOfWQnc8g@mail.gmail.com>
On Fri, 07 Aug 2020, Gene Chen wrote:
> Mark Brown <broonie@kernel.org> 於 2020年8月6日 週四 下午8:13寫道:
> >
> > On Thu, Aug 06, 2020 at 11:30:56AM +0800, Gene Chen wrote:
> > > Mark Brown <broonie@kernel.org> 於 2020年8月6日 週四 上午12:10寫道:
> >
> > > > It's not clear why this isn't just done in the device regmap, there's
> > > > exactly one user?
> >
> > > because I use one regmap to access 4 I2C devices,
> >
> > There appears to be only one device here?
> >
> > > I need change the regmap_bus struct to fit I2C read/write with CRC bit
> > > Therefore, MFD reviewer suggests I can move the regmap api to regmap
> > > folder such as regmap-ac97.c
> >
> > AC'97 is an industry standard bus used by a range of devices in
> > different subsystems. You can already have custom operations for a
> > device just in a regular regmap using the reg_read() and reg_write()
> > operations which are there so devices that individual device support
> > doesn't need to be added to the regmap core.
> >
>
> I need use regmap_raw_read to access MT6360 TYPEC part, so we need
> implement bus read control
>
> > You really also need to write a much clearer changelog, I would be hard
> > pressed to tell from the changelog that this was moving things to the
> > regmap core rather than shuffling regmaps within the device.
>
> MT6360 has 4 I2C worker devices
> First, I increase reg_bits from 8 to 16 bits.
> Higher 8 bits, bank, indicated which worker device I want access
> Then, if worker devices is PMIC or LDO part, I need calculate or check
> CRC8 bits when we write or read data.
> CRC8 bits is calculated by 3 parts.
> 1'st part include 1 byte is worker address and R/W in LSB.
> 2'nd part include 1 byte is register address
> 3'nd part include written data or read data from MT6360
> I also need 1 dummy byte when write data
>
> @Lee Jones,
> I found out drivers/iio/chemical/bme680_spi.c implement their own
> regmap_bus too.
> Can I move regmap control back to mt6360-core.c?
Yes, if that is the 'done thing'.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: Gene Chen <gene.chen.richtek@gmail.com>
Cc: Mark Brown <broonie@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
rafael@kernel.org, gregkh@linuxfoundation.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
Gene Chen <gene_chen@richtek.com>,
benjamin.chao@mediatek.com, shufan_lee@richtek.com,
cy_huang@richtek.com
Subject: Re: [PATCH 9/9] mfd: mt6360: Merge different sub-devices I2C read/write
Date: Mon, 10 Aug 2020 08:08:43 +0100 [thread overview]
Message-ID: <20200810070843.GA4411@dell> (raw)
In-Reply-To: <CAE+NS37tr65GnTue89wJkPvJzddahKj__KPgzmjzwkOfWQnc8g@mail.gmail.com>
On Fri, 07 Aug 2020, Gene Chen wrote:
> Mark Brown <broonie@kernel.org> 於 2020年8月6日 週四 下午8:13寫道:
> >
> > On Thu, Aug 06, 2020 at 11:30:56AM +0800, Gene Chen wrote:
> > > Mark Brown <broonie@kernel.org> 於 2020年8月6日 週四 上午12:10寫道:
> >
> > > > It's not clear why this isn't just done in the device regmap, there's
> > > > exactly one user?
> >
> > > because I use one regmap to access 4 I2C devices,
> >
> > There appears to be only one device here?
> >
> > > I need change the regmap_bus struct to fit I2C read/write with CRC bit
> > > Therefore, MFD reviewer suggests I can move the regmap api to regmap
> > > folder such as regmap-ac97.c
> >
> > AC'97 is an industry standard bus used by a range of devices in
> > different subsystems. You can already have custom operations for a
> > device just in a regular regmap using the reg_read() and reg_write()
> > operations which are there so devices that individual device support
> > doesn't need to be added to the regmap core.
> >
>
> I need use regmap_raw_read to access MT6360 TYPEC part, so we need
> implement bus read control
>
> > You really also need to write a much clearer changelog, I would be hard
> > pressed to tell from the changelog that this was moving things to the
> > regmap core rather than shuffling regmaps within the device.
>
> MT6360 has 4 I2C worker devices
> First, I increase reg_bits from 8 to 16 bits.
> Higher 8 bits, bank, indicated which worker device I want access
> Then, if worker devices is PMIC or LDO part, I need calculate or check
> CRC8 bits when we write or read data.
> CRC8 bits is calculated by 3 parts.
> 1'st part include 1 byte is worker address and R/W in LSB.
> 2'nd part include 1 byte is register address
> 3'nd part include written data or read data from MT6360
> I also need 1 dummy byte when write data
>
> @Lee Jones,
> I found out drivers/iio/chemical/bme680_spi.c implement their own
> regmap_bus too.
> Can I move regmap control back to mt6360-core.c?
Yes, if that is the 'done thing'.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2020-08-10 7:09 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-04 16:32 [PATCH v3 0/9] mfd: mt6360: Merge different sub-devices I2C read/write Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` [PATCH 1/9] mfd: mt6360: Rearrange include file Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` [PATCH 2/9] mfd: mt6360: Remove redundant brackets around raw numbers Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` [PATCH 3/9] mfd: mt6360: Indicate sub-dev compatible name by using "-" Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` [PATCH 4/9] mfd: mt6360: Combine mt6360 pmic/ldo resources into mt6360 regulator resources Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` [PATCH 5/9] mfd: mt6360: Rename mt6360_pmu_data by mt6360_data Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` [PATCH 5/9] mfd: mt6360: Rename mt6360_pmu_data by mt6360_ddata Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` [PATCH 6/9] mfd: mt6360: Rename mt6360_pmu by mt6360 Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:32 ` Gene Chen
2020-08-04 16:33 ` [PATCH 7/9] mfd: mt6360: Remove handle_post_irq callback function Gene Chen
2020-08-04 16:33 ` Gene Chen
2020-08-04 16:33 ` Gene Chen
2020-08-04 16:33 ` [PATCH 8/9] mfd: mt6360: Fix flow which is used to check ic exist Gene Chen
2020-08-04 16:33 ` Gene Chen
2020-08-04 16:33 ` Gene Chen
2020-08-04 16:33 ` [PATCH 9/9] mfd: mt6360: Merge different sub-devices I2C read/write Gene Chen
2020-08-04 16:33 ` Gene Chen
2020-08-04 16:33 ` Gene Chen
2020-08-05 16:10 ` Mark Brown
2020-08-05 16:10 ` Mark Brown
2020-08-05 16:10 ` Mark Brown
2020-08-06 3:30 ` Gene Chen
2020-08-06 3:30 ` Gene Chen
2020-08-06 3:30 ` Gene Chen
2020-08-06 12:13 ` Mark Brown
2020-08-06 12:13 ` Mark Brown
2020-08-06 12:13 ` Mark Brown
2020-08-07 2:16 ` Gene Chen
2020-08-07 2:16 ` Gene Chen
2020-08-07 2:16 ` Gene Chen
2020-08-07 11:13 ` Mark Brown
2020-08-07 11:13 ` Mark Brown
2020-08-07 11:13 ` Mark Brown
2020-08-10 7:08 ` Lee Jones [this message]
2020-08-10 7:08 ` Lee Jones
2020-08-10 7:08 ` Lee Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200810070843.GA4411@dell \
--to=lee.jones@linaro.org \
--cc=benjamin.chao@mediatek.com \
--cc=broonie@kernel.org \
--cc=cy_huang@richtek.com \
--cc=gene.chen.richtek@gmail.com \
--cc=gene_chen@richtek.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=rafael@kernel.org \
--cc=shufan_lee@richtek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.