From: Lee Jones <lee.jones@linaro.org>
To: wwang <wei_wang@realsil.com.cn>
Cc: sameo@linux.intel.com, devel@linuxdriverproject.org,
linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org,
rogerable@realtek.com, micky_ching@realsil.com.cn
Subject: Re: [PATCH v3] mfd: rtsx: Modify rts5249_optimize_phy
Date: Fri, 13 Sep 2013 08:28:03 +0100 [thread overview]
Message-ID: <20130913072803.GT11227@lee--X1> (raw)
In-Reply-To: <522FC82C.4020000@realsil.com.cn>
> >I'm not asking for in-depth analysis, just an overview.
> >
> >What's wrong with the default config?
> >Why is the signal quality bad and what makes it bad?
> >What did the old magic numbers do?
> >How will the configuration differ if I applied your patch?
I'm not sure I'm getting the answers I crave here.
> It is a little different between simulation and real chip. We have
> no idea about which configuration is better before tape-out. We set
> default settings according to simulation, but need to tune these
> parameters after getting the real chip.
What parameters? I can see they've changed, but what were they before?
> Those old magic numbers are proper in simulation environment, but
> not good in real ASIC chip.
I'm sure they suited at some point or they wouldn't have been used,
but what's the difference between the old magic numbers and the new
defines? 0xFE46 tells me nothing. I can't make a comparison without
you telling me, in words, in the commit log what you're actually
doing.
> If the new patch won't be applied, we may failed to access SD card
> in those platforms equipped with rts5249 module.
Fine. I get that. I have no problem applying the patch, but I'd really
like to know what it is that you're changing before blindly doing so.
Other changes which deserve a little explaining:
Why don't you set the PHY_BPCR anymore? 0x05C0 looks like quite a bit
of configuration. What was it and why isn't it required anymore?
And why is there a need to set the; PHY_PCR, PHY_RCR2, PHY_FLD4,
PHY_RDR, PHY_RCR1, PHY_FLD3 and PHY_TUNE registers where there
wasn't this requirement before?
You don't need to reply to this message. Write a proper, informative
commit log into the patch and resubmit. If I'm satisfied I know what
you're actually adapting in the configuration I'll have no issue and
apply the patch - the code looks good.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
prev parent reply other threads:[~2013-09-13 7:28 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-10 9:13 [PATCH v3] mfd: rtsx: Modify rts5249_optimize_phy wei_wang
2013-09-10 9:28 ` Lee Jones
2013-09-10 9:58 ` wwang
2013-09-10 11:08 ` Lee Jones
2013-09-11 1:32 ` wwang
2013-09-13 7:28 ` Lee Jones [this message]
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=20130913072803.GT11227@lee--X1 \
--to=lee.jones@linaro.org \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=micky_ching@realsil.com.cn \
--cc=rogerable@realtek.com \
--cc=sameo@linux.intel.com \
--cc=wei_wang@realsil.com.cn \
/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