Hi,
No, it does not look OK.
1) it does not indicate how delay is long actually anywhere. He does delay(300) but what is the value 300? is it ms or just the price of wood in Alaska? (= The delay loop must have enough time as it described in datasheet. I do not know: is it correct value in this program depend on used MCU, frequence, X2 mode etc?
2) WriteLCD() has limited with 20 symbols (okay, I think it is due LCD line limit). Why then author does WriteLCD( "DEPARTMENT OF ELECTRONICS" ); which has much more signs and there are also some extra WriteData() following (!!!) this line.
3) some commands need much more time to be executed, especialy commands which clear display and it`s DRAM. In InitLCD() all the commands are executed with delay(300) - huh, is it magic number anywhere?
4) CursorHome() is called twice - the command 0x02 is already included inside InitLCD();
5) Depend on compiler and optimization level: user should declare variable with volatile keyword. Read C51 manual of Keil with node Absolute Variable Location - there is special note about it there:
NOTE
If you use the _at_ keyword to declare a variable that accesses an XDATA peripheral, you may require the volatile keyword to ensure that the C compiler does not optimize out necessary memory accesses.
Erhm, here are some mistakes I have found on first look. Probably there are some more.
Good days!