From mboxrd@z Thu Jan 1 00:00:00 1970 From: Timur Tabi Date: Thu, 17 Jan 2008 11:39:21 -0600 Subject: [U-Boot-Users] [PATCH v2] QE IO: Add initial data to pin configuration + read/write functions In-Reply-To: References: Message-ID: <478F92C9.8030205@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de David Saada wrote: > @@ -38,6 +38,16 @@ void qe_config_iopin(u8 port, u8 pin, in > volatile par_io_t *par_io = (volatile par_io_t *) > &(gur->qe_par_io); > > + /* Calculate pin location for 1bit mask */ > + pin_1bit_mask = (u32)(1 << (NUM_OF_PINS - (pin+1))); > + > + /* Setup the data */ > + tmp_val = in_be32(&par_io[port].cpdat); > + if (data) > + out_be32(&par_io[port].cpdat, pin_1bit_mask | tmp_val); > + else > + out_be32(&par_io[port].cpdat, ~pin_1bit_mask & tmp_val); > + I see that qe_config_iopin() will always write a 0 or 1 now, ignoring the current value. Are you sure this is a good idea? What if I don't want U-Boot to change the current pin value? Plus, doesn't this assume that the pin is set to output? Shouldn't you check to see if the pin is output or input/output, and only write cpdat if it is? Also, I believe that cpdat is used only if the pin is configured as a GPIO. If so, I don't see you check for that, either. -- Timur Tabi Linux kernel developer at Freescale