From: Arseny Solokha <asolokha@kb.kras.ru>
To: Wolfram Sang <wsa@the-dreams.de>, Arseny Solokha <asolokha@kb.kras.ru>
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
Valentin Longchamp <valentin.longchamp@keymile.com>
Subject: Re: [RESEND, 4/4] i2c: mpc: always determine I2C clock prescaler at runtime
Date: Wed, 10 Jan 2018 18:36:00 +0700 [thread overview]
Message-ID: <a218a553-4d6b-be21-389e-037fd7838277@kb.kras.ru> (raw)
In-Reply-To: <20171230181446.bxongiaogcqjrbos@ninjato>
>> /*
>> * Map and check POR Device Status Register 2
>> - * (PORDEVSR2) at 0xE0014
>> + * (PORDEVSR2) at 0xE0014. Note than while MPC8533
>> + * and MPC8544 indicate SEC frequency ratio
>> + * configuration as bit 26 in PORDEVSR2, other MPC8xxx
>> + * parts may store it differently or may not have it
>> + * at all.
>
> So given this comment which you added...
>
>> */
>> reg = ioremap(get_immrbase() + *prop + 0x14, 0x4);
>> if (!reg)
>> printk(KERN_ERR
>> "Error: couldn't map PORDEVSR2\n");
>> else
>> - val = in_be32(reg) & 0x00000080; /* sec-cfg */
>> + val = in_be32(reg) & 0x00000020; /* sec-cfg */
>
> ... are you really sure there is no ancient device which needs the
> 0x00000080?
Various MPC SoCs indeed have different bit layout of
PORDEVSR2. Obviously not all of them indicate SEC frequency ratio
configuration at all, either in PORDEVSR2 or in some other register, as
not all SoCs contain SEC engine. In this regard,
mpc_i2c_get_sec_cfg_8xxx() is poorly named as it in its current form is
only applicable to a few SoCs from the whole MPC8xxx family.
mpc_i2c_get_sec_cfg_8xxx() is only called from the following snippet:
else if ((SVR_SOC_VER(svr) == SVR_8533)
|| (SVR_SOC_VER(svr) == SVR_8544))
/* the above 85xx SoCs have prescaler 3 or 2 */
prescaler = mpc_i2c_get_sec_cfg_8xxx() ? 3 : 2;
Both MPC8533 and MPC8544 do in fact indicate SEC frequency ratio
configuration as bit 26 in PORDEVSR2 according to [1,2], so I've added
the comment as a precaution for future uses. I can also rename the
function to something like mpc_i2c_get_sec_cfg_8533_8544(), which looks
ugly and doesn't scale.
AFAICS, mask 0x00000080 in the code clearly contradicts to what I read
in the docs.
[1] https://www.nxp.com/docs/en/reference-manual/MPC8533ERM.pdf
[2] https://www.nxp.com/docs/en/reference-manual/MPC8544ERM.pdf
Arsény
next prev parent reply other threads:[~2018-01-10 11:36 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-10 7:50 [PATCH 0/4] i2c: mpc: Clean up clock selection Arseny Solokha
2017-11-10 7:50 ` [PATCH 1/4] i2c: mpc: get MPC8xxx I2C clock prescaler before using it in calculations Arseny Solokha
2017-11-10 7:50 ` [PATCH 2/4] i2c: mpc: unify obtaining the MPC8533/44 I2C clock prescaler w/ MPC8xxx Arseny Solokha
2017-11-10 7:50 ` [PATCH 3/4] i2c: mpc: fix PORDEVSR2 mask for MPC8533/44 Arseny Solokha
2017-11-10 7:50 ` [PATCH 4/4] i2c: mpc: always determine I2C clock prescaler at runtime Arseny Solokha
2017-12-07 10:19 ` [PATCH RESEND 0/4] i2c: mpc: Clean up clock selection Arseny Solokha
2017-12-07 10:20 ` [PATCH RESEND 1/4] i2c: mpc: get MPC8xxx I2C clock prescaler before using it in calculations Arseny Solokha
2017-12-30 18:18 ` [RESEND, " Wolfram Sang
2018-01-15 18:21 ` Wolfram Sang
2017-12-07 10:20 ` [PATCH RESEND 2/4] i2c: mpc: unify obtaining the MPC8533/44 I2C clock prescaler w/ MPC8xxx Arseny Solokha
2017-12-30 18:19 ` [RESEND, " Wolfram Sang
2018-01-15 18:21 ` Wolfram Sang
2017-12-07 10:20 ` [PATCH RESEND 3/4] i2c: mpc: fix PORDEVSR2 mask for MPC8533/44 Arseny Solokha
2017-12-30 18:11 ` [RESEND,3/4] " Wolfram Sang
2018-01-15 18:22 ` Wolfram Sang
2017-12-07 10:20 ` [PATCH RESEND 4/4] i2c: mpc: always determine I2C clock prescaler at runtime Arseny Solokha
2017-12-30 18:14 ` [RESEND, " Wolfram Sang
2018-01-10 11:36 ` Arseny Solokha [this message]
2018-01-10 11:36 ` [PATCH v2 " Arseny Solokha
2018-01-15 18:22 ` Wolfram Sang
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=a218a553-4d6b-be21-389e-037fd7838277@kb.kras.ru \
--to=asolokha@kb.kras.ru \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=valentin.longchamp@keymile.com \
--cc=wsa@the-dreams.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®